dev: update docker node version, server address, and docker-compose #3053
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @carols10cents (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
r? @jtgeibel |
.env.sample
Outdated
| @@ -1,3 +1,6 @@ | |||
| # Toggle for server configuration in development when using Docker. | |||
| export DEV_DOCKER=1 | |||
There was a problem hiding this comment.
I don't think this needs to be added to the sample file, as this shouldn't be enabled for native local development. Shouldn't this be added directly to the docker-compose.yml file instead?
There was a problem hiding this comment.
@jtgeibel - my thinking here was that since the .env file is read by the dockerized environment, it is a good place to add the toggle, instead of the docker-compose.yml, which could be used for hosting. If it is used while hosted, then we've exposed the port too widely.
I updated my branch using the flag directly within docker-compose.yml at first, and I don't mind reverting it. Just figured it would be more flexible this way.
There was a problem hiding this comment.
@nilslice I'm not sure I follow. If docker-compose.yml is used for deployment, then wouldn't this environment variable also need to be set for that to work?
At the least, we should drop this from .env.sample and mention it to .env only under the docker section of CONTRIBUTING.md. That is the only scenario where we want to recommend using this variable.
There was a problem hiding this comment.
I've taken the liberty to rebase the branch and perform the change to the docker-compose.yml file, since I was considering working on #2054, but then found this PR here 😅
should be good to go from my side 👍
Turbo87
force-pushed
the
sm/docker-run-fix
branch
from
027bc36 to
a124906
Compare
to make the file a bit easier on the eyes ;)
Turbo87
force-pushed
the
sm/docker-run-fix
branch
from
dfc5b2c to
324bf5e
Compare
|
@bors r+ |
|
📌 Commit 324bf5e has been approved by |
|
☀️ Test successful - checks-actions |
|
Great, thanks!! |
Fixes dockerized environment to run crates.io locally.