Use env vars to set port; respect env vars in docker compose #6085
Conversation
|
Heroku app: https://gyr-review-app-6085-36b3ce6c2d01.herokuapp.com/ |
| cask "temurin@21" | ||
| brew "poppler" | ||
| brew "postgresql@14" | ||
| brew "postgresql" |
There was a problem hiding this comment.
It may be that we want to remove this line. We require binaries though. We can get them from libpq instead but it requires a modification of PATH which we don't have a standard tool to do. I recommend mise if we proceed down that path.
There was a problem hiding this comment.
I think this is as reasonable as anything else if there isn't a brew formula for the exact binaries we need.
| system! "docker compose pull db" | ||
| system! "docker compose up --wait -d db" |
There was a problem hiding this comment.
No longer stopping Postgres because other projects might be using it. Should be sufficient to just start on another port. This up call should stop and restart if env vars change and assign the new ports. pull to check for updates to the specified image before uping.
| username: <%= ENV.fetch("RAILS_DB_USERNAME", 'postgres') %> | ||
| password: <%= ENV.fetch("RAILS_DB_PASSWORD", 'password') %> |
There was a problem hiding this comment.
Don't need this after using .env. Shouldn't have made it in.
There was a problem hiding this comment.
Not sure what "shouldn't have made it in" means here & just want to double check to make sure I'm not missing something - these lines were added by Melanie when she added the docker compose file
There was a problem hiding this comment.
I added these lines in my last merge. They're unnecessary because they only matter in development and we're already using .env in development so we should be dealing with it there.
There was a problem hiding this comment.
my bad - i was looking at the very similar lines 7-8 up in local_default
There was a problem hiding this comment.
does the same rationale for removing these lines apply now to those ones?
There was a problem hiding this comment.
I think I'm inclined to leave them alone. Ports rarely change and username / password additionally are allowable to be entirely nil. This differs from setting them to '' because it switches postgres into trust authentication, if enabled.
| user: postgres | ||
| ports: | ||
| - 127.0.0.1:5432:5432 | ||
| - 127.0.0.1:${RAILS_DB_PORT:-5432}:5432 |
There was a problem hiding this comment.
The unusual notation of ${SOME_ENV_VAR:-value} allows you to set value as a default value when SOME_ENV_VAR is not set or otherwise empty.
|
@powersurge360 Was there a change to the |
|
You are correct! Thanks. |
|
Ok, with a change to my |
Link to pivotal/JIRA issue
Is PM acceptance required? (delete one)
Reminder: merge main into this branch and get green tests before merging to main
What was done?
.envto reflect an incremented portBrewfilefor postgresHow to test?
.envto include a newRAILS_DB_PORT(find a suggested port indotenv)bin/setupdocker compose ps