Brewfile for dependency management#8475
Merged
night-jellyfish merged 4 commits intomainfrom May 30, 2023
Merged
Conversation
aduth
reviewed
May 24, 2023
*Summary of changes* We're looking at adding a dependency soon, which could cause some issues with local development for folks if they didn't know they need to manually install it. We thought adding a `Brewfile` and having the setup script run `brew bundle` would help mitigate the difficulties of that change. There's a bonus in that it's self documenting too! It would also: - make initial setup for new developers much more straightforward - make future changes to dependencies easier to manage - prevents a mixture of dependencies installed in different ways (for instance, I think when I initially set things up I ended up with one version of postgres downloaded manually from the doc-linked site, and another via homebrew) Notes: - As pointed out by @pauldoomgov, we use [postgres@13 in production](https://github.com/18F/identity-devops/blob/33ba02736b37f3a79c50479892a03c6f3e920041/terraform/app/rds-variables.tf#L82), so it would be good to encourage use of 13 locally. The method I chose to try and enforce this was using `brew services stop --all` before starting the services we want (where I specify 13). - I tried various combinations of having these dependencies installed / uninstalled before running `make setup` and the script seems to still work. But I don't have a machine without MacOS to test on to make sure it works for everyone. - While making relevant doc changes, I also fixed up some syntax for readability / easy re-ordering of steps
night-jellyfish
force-pushed
the
brittany/brewfile_for_dependency_management
branch
from
d441f5c to
7566352
Compare
aduth
reviewed
May 30, 2023
zachmargolis
approved these changes
May 30, 2023
Contributor
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM. I tried it out and it ended up downgrading me from postgres 14 to postgres 13, but it did work! I forget what version we are running in prod
| @@ -0,0 +1,6 @@ | |||
| brew 'postgresql@13' | |||
Contributor
There was a problem hiding this comment.
somehow I was on postgres 14 (which I think is what matches prod right?)
Contributor
Author
There was a problem hiding this comment.
Thanks for trying it out, @zachmargolis ! 🎉 I'm glad it worked for you.
We are actually using postgres@13 in production. In the Slack thread regarding this ticket, it was mentioned that we may be moving to 14 in the coming months but are not there yet.
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
night-jellyfish
pushed a commit
that referenced
this pull request
May 31, 2023
In #8475, we chose to keep Postgres at version 13 so that it could [match production](https://github.com/18F/identity-devops/blob/33ba02736b37f3a79c50479892a03c6f3e920041/terraform/app/rds-variables.tf#L82). However, when trying to continue on the `postgis` work, I found that when installing `postgis`, `brew` installs 14 anyway, as a dependency of `postgis`. There doesn't seem to be an easy way to tell Brew not to install 14, or postgis to use 13 instead. This becomes an issue then when the app is using 13, but `postgis` is using 14. I thought it'd be better to change this now than wait for the postgis work to be ready, as it would potentially stop folks from installing an extra version of postgres. There may be another workaround, but after reading [a homebrew issue about versioning issues with postgres and postgis](Homebrew/homebrew-core#86709), which included suggestions like [making your own copy of `postgis` and changing its dependencies](Homebrew/homebrew-core#86709 (comment)), it seemed like upgrading `postgres` was a better solution. While it will mean dev is on a different version than prod, most folks have been using 14 or even 15, so we don't expect issues to arise.
Merged
night-jellyfish
added a commit
that referenced
this pull request
Jun 1, 2023
changelog: Internal, Application dependencies, Upgrade postgres to version 14 In #8475, we chose to keep Postgres at version 13 so that it could [match production](https://github.com/18F/identity-devops/blob/33ba02736b37f3a79c50479892a03c6f3e920041/terraform/app/rds-variables.tf#L82). However, when trying to continue on the `postgis` work, I found that when installing `postgis`, `brew` installs 14 anyway, as a dependency of `postgis`. There doesn't seem to be an easy way to tell Brew not to install 14, or postgis to use 13 instead. This becomes an issue then when the app is using 13, but `postgis` is using 14. I thought it'd be better to change this now than wait for the postgis work to be ready, as it would potentially stop folks from installing an extra version of postgres. There may be another workaround, but after reading [a homebrew issue about versioning issues with postgres and postgis](Homebrew/homebrew-core#86709), which included suggestions like [making your own copy of `postgis` and changing its dependencies](Homebrew/homebrew-core#86709 (comment)), it seemed like upgrading `postgres` was a better solution. While it will mean dev is on a different version than prod, most folks have been using 14 or even 15, so we don't expect issues to arise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠 Summary of changes
We're looking at adding a dependency soon, which could cause some issues
with local development for folks if they didn't know they need to
manually install it. We thought adding a
Brewfileand having the setupscript run
brew bundlewould help mitigate the difficulties of thatchange. There's a bonus in that it's self documenting too!
It would also:
instance, I think when I initially set things up I ended up with one
version of postgres downloaded manually from the doc-linked site, and
another via homebrew)
Notes:
so it would be good to encourage use of 13 locally. The method I chose
to try and enforce this was using
brew services stop --allbeforestarting the services we want (where I specify 13).
uninstalled before running
make setupand the script seems to stillwork. But I don't have a machine without MacOS to test on to make sure
it works for everyone.
readability / easy re-ordering of steps