Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Devenv 1.x #95

Merged
merged 2 commits into from
Apr 9, 2024
Merged

Devenv 1.x #95

merged 2 commits into from
Apr 9, 2024

Conversation

schneider-felix
Copy link
Contributor

1. Why is this change necessary?

These changes ensure compatibility with devenv 1.x

2. What does this change do, exactly?

I disabled automatic ca installation for caddy as this prevented the server from starting up when using tls. Furthermore I removed the usage of currentSystem because it was removed in devenv version 1.0

3. Describe each step to reproduce the issue or behaviour.

4. Please link to the relevant issues (if any).

Fixes #94

5. Checklist

  • I have rebased my changes to remove merge conflicts
  • I have written or adjusted the documentation according to my changes

@M-arcus
Copy link
Contributor

M-arcus commented Mar 29, 2024

Please add process.implementation = "honcho";, the new process manager blocks the port 9999 if not configured otherwise

@schneider-felix
Copy link
Contributor Author

schneider-felix commented Mar 31, 2024

@M-arcus is this strictly necessary for compatibility reasons or is this a general recommendation for 1.x?

@M-arcus
Copy link
Contributor

M-arcus commented Mar 31, 2024

Yes, without further configuration this does not allow starting 2 devenvs of this setup at the same time. Alternatively there needs to be a configuration added to be able to access the process -compose port.

@schneider-felix
Copy link
Contributor Author

@M-arcus whats does process manager use Port 9999 for? Are there any downsides to changing this config?

@M-arcus
Copy link
Contributor

M-arcus commented Mar 31, 2024

It's the port of the rest client of process-compose: https://f1bonacc1.github.io/process-compose/client/?h=port#rest-api

Honcho does not have one by default, therefore no port conflicts

Devenv .6 had honcho, devenv 1 has honcho, process-compose, overmind and hivemend as process manager

@lx-wnk
Copy link
Contributor

lx-wnk commented Apr 3, 2024

Thanks for the explanation @M-arcus.

Thank you for your contribution @schneider-felix!

Personally, I'd say that we should go with the default values, especially since with our configuration it is not possible to start two projects without adjusting the ports anyway.

However, this should be explicitly mentioned in the README and the update instructions.

What do you think?

@M-arcus
Copy link
Contributor

M-arcus commented Apr 3, 2024

Adhering to the defaults is important, I agree 👍

In case this PR gets merged in the current state, I will add a PR with an (optional) port configuration for process-compose, since it's probably only affecting a few users.

@kleinmann kleinmann merged commit 7141799 into kellerkinderDE:main Apr 9, 2024
3 checks passed
@kleinmann
Copy link
Member

Thank you for your contribution @schneider-felix ! 🙂

@M-arcus Please feel free to add the info for process-compose ports 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error: attribute 'currentSystem' missing while building
4 participants