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

Use alembic for database table creation #298

Merged
merged 31 commits into from
Feb 16, 2024

Conversation

jstutters
Copy link
Contributor

@jstutters jstutters commented Feb 12, 2024

Allows us to be able to control the upgrade of the pixl database tables in production with upgrades and rollbacks

@jstutters jstutters requested a review from a team February 12, 2024 17:30
@jstutters jstutters self-assigned this Feb 12, 2024
@jstutters jstutters added this to the VOXL milestone Feb 13, 2024
@jstutters jstutters marked this pull request as ready for review February 13, 2024 09:31
Copy link
Contributor

@stefpiatek stefpiatek left a comment

Choose a reason for hiding this comment

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

Really nice, thanks for your speedy work on this. I think we want to have this run when the imaging-api starts up because the postgres init db script only runs if there is no database. That way we can update the schema without losing any data

docker/postgres/Dockerfile Outdated Show resolved Hide resolved
postgres/pixl-db_init.sh Outdated Show resolved Hide resolved
alembic/README Outdated Show resolved Hide resolved
stefpiatek
stefpiatek previously approved these changes Feb 14, 2024
Copy link
Contributor

@stefpiatek stefpiatek left a comment

Choose a reason for hiding this comment

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

Code changes look perfect, assuming it runs! I think there's also some documentation we could add to the alembic README. Happy for you to merge once you're happy with it

@stefpiatek stefpiatek linked an issue Feb 14, 2024 that may be closed by this pull request
1 task
@stefpiatek stefpiatek dismissed their stale review February 14, 2024 17:42

Picked this up so my review shouldn't count

@stefpiatek stefpiatek requested a review from a team February 14, 2024 17:54
@stefpiatek stefpiatek self-assigned this Feb 15, 2024
Copy link
Member

@milanmlft milanmlft left a comment

Choose a reason for hiding this comment

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

Not really familiar with Alembic, so probably not the best judge but it looks good to me.
A bit more documentation would be nice though 😬

postgres/README.md Outdated Show resolved Hide resolved
@stefpiatek stefpiatek requested a review from milanmlft February 15, 2024 18:10
@stefpiatek stefpiatek requested a review from a team February 15, 2024 18:23
Copy link
Contributor

@peshence peshence 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, huge improvement once it's over the line. Couple of small comments/questions here and there.

Also, shouldn't this be part of the core module?

pixl_imaging/alembic/autogenerate-migration.sh Outdated Show resolved Hide resolved
pixl_imaging/README.md Outdated Show resolved Hide resolved
@stefpiatek stefpiatek enabled auto-merge (squash) February 16, 2024 17:22
@stefpiatek stefpiatek merged commit 382f16c into main Feb 16, 2024
8 checks passed
@stefpiatek stefpiatek deleted the jstutters/270-use-alembic-for-database-table-creation branch February 16, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow PIXL DB tables to be updated in production
4 participants