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

Upgrade postgres to 14.10 in dev container setup #1059

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

lognaturel
Copy link
Member

@lognaturel lognaturel commented Dec 8, 2023

Follow on to #1052

I wanted the changes that affect production to go on staging as quickly as possible so left this behind. At the time I was having trouble running the make task because I already had a postgres server running on my machine. The make task would start the container but then would fail when trying to create the jubilant role because it already existed in my local db.

I've also switched to the alpine image. I don't think we need any particular utilities to be available and this saves ~40mb in download.

What has been done to verify that this works as intended?

Discarded my existing container and image, ran the make task, verified I could use the db with Central and that the postgres version was 14.10.

Why is this the best possible solution? Were any other approaches considered?

This is not really a complete solution because it won't upgrade an existing container. It seems like a reasonable first step but I think we should give some more thought to what we want this task to do:

  • If the goal is to quickly spin up a clean database, then this is probably fine. Folks who use it will periodically throw away their existing containers and images and create new ones which will have the latest postgres version.
  • If the goal is to have a stable local database for development, I think we should use a named volume and make it so containers can be upgraded

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

None.

Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.

No

Before submitting this PR, please make sure you have:

  • run make test-full and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code from external sources are properly credited in comments or that everything is internally sourced

@lognaturel lognaturel marked this pull request as ready for review December 8, 2023 19:41
@lognaturel lognaturel requested a review from ktuite December 8, 2023 19:41
@lognaturel
Copy link
Member Author

Using the alpine-based image currently will not match production. I think that's ok but I wanted to mention it. The benefit to using alpine here is that it's a smaller download for someone who just want the postgres container.

I initially thought it made sense to switch the base image for production to the alpine variant but I realized that would mean downloading totally new layers instead of using several that are likely shared with the node image. I could be convinced that this consideration is an overoptimization and that we should just go for the smaller postgres image in production. Again, I think this change can be considered independently but wanted to provide context.

Copy link
Member

@ktuite ktuite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I tried it (following the path of just deleting my existing dev DB) and now I'm onto this fresh new database.

I like the simplicity of the first goal and that's how I seem to use it:

If the goal is to quickly spin up a clean database, then this is probably fine. Folks who use it will periodically throw away their existing containers and images and create new ones which will have the latest postgres version.

It's just a helpful little utility! And the less is baked into it, the less extra stuff someone needs to load into their brain. If someone needs their data to persist, they can choose their favorite option

  • run postgres some other way
  • don't run this command to remake the postgres dev container
  • use other database (or central) backup/restore techniques

@lognaturel lognaturel merged commit 46e1cc3 into master Dec 8, 2023
5 checks passed
@lognaturel lognaturel deleted the postgres-dev-docker branch December 8, 2023 23:52
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.

2 participants