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.

Sign up for GitHub

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

Pin alembic version to 'alembic==1.9.4' #354

Merged
merged 2 commits into from
Mar 9, 2023
Merged

Pin alembic version to 'alembic==1.9.4' #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42bf446
Pin alembic version to 'alembic==1.9.4'
dlpzx Mar 7, 2023
258481a
Pin alembic version in dbmigration CodeBuild in infra account
dlpzx Mar 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ assume-role:
rm .assume_role_json

drop-tables: upgrade-pip install-backend
pip install alembic
pip install 'alembic==1.9.4'
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a heads up - 1.10.1 was released just a couple of days ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, on March 6th they released 1.10.1 (data.all pipeline was working fine on March 5th) which included some regression fix that seems related to our issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @kukushking I have tested it in a deployment and the migration completes successfully (both in the validate migration step and in the actual migration).

What I was thinking is that instead of pinning the version directly in the CodeBuild command, we could add alembic as a backend package in backend/requirements.txt so that it is treated as any other package. The only downside that I see is that alembic is not a strictly requirement of the backend compute resources. What is your opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's all right, no strong opinion on this one. As long as there is a comment specifying why it's there. Also considering migration script might be called outside CB.

export PYTHONPATH=./backend && \
python backend/migrations/drop_tables.py

upgrade-db: upgrade-pip install-backend
pip install alembic
pip install 'alembic==1.9.4'
export PYTHONPATH=./backend && \
alembic -c backend/alembic.ini upgrade head

Expand Down
2 changes: 1 addition & 1 deletion deploy/stacks/dbmigration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def __init__(
'python -m venv env',
'. env/bin/activate',
'pip install -r backend/requirements.txt',
'pip install alembic',
'pip install "alembic==1.9.4"',
'export PYTHONPATH=backend',
f'export envname={envname}',
f'alembic -c backend/alembic.ini upgrade head',
Expand Down