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

DevContainer QoL change #96

Merged
merged 1 commit into from
Feb 10, 2024

Conversation

jacobsee
Copy link
Contributor

@jacobsee jacobsee commented Jan 5, 2024

We pass several ports from this devcontainer compose stack (NATS, Hydra, CRDB...) through to the host using the ports option directly in the docker-compose.yml file... but we have been omitting the port used by the governor-api app itself from the compose file and instead forwarded it using the devcontainer.json forwardPorts definition.

This works seamlessly in VSCode, however, forwardPorts is unsupported by the devcontainer CLI for anyone using another editor/IDE. The main difference between forwarding a port in the compose file vs. using forwardPorts is that forwardPorts is necessary for services that only listen on 127.0.0.1 - however, this service listens on 0.0.0.0, thus there is no functional difference here, so proposing this change for QoL/compatibility reasons.

@jacobsee jacobsee requested a review from a team as a code owner January 5, 2024 05:10
@jacobsee jacobsee force-pushed the small-devcontainer-changes branch from ace32ce to 5e0cc37 Compare February 4, 2024 05:20
@jacobsee jacobsee requested a review from jnschaeffer February 7, 2024 19:42
@jacobsee jacobsee enabled auto-merge (squash) February 9, 2024 22:12
…We aren't making use of the differences in devcontainer 'forwardPorts
@jacobsee jacobsee force-pushed the small-devcontainer-changes branch from 5e0cc37 to 831b33f Compare February 10, 2024 00:30
@jacobsee jacobsee merged commit be6f3dc into metal-toolbox:main Feb 10, 2024
1 check was pending
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.

3 participants