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

fix: Rewrite the vfolder status migration in #1892 #1936

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented Feb 28, 2024

The migration in #1892 requires a manual alembic stamp command after running the alembic upgrade command. This will break many developers and fieldops members.

This PR migrates the vfolders.status enum column to a VARCHAR(64) column to avoid complex enum value substitution and simplify the migration process.

  • fix: Rewrite the vfolder status migration (7ff52ff68bfc_detail_vfolder_deletion_status.py)
    • Remove errorneous commit during alembic migration scripts
    • Use jsonb functions to convert the key names in the status_history column at once
  • refactor: Add models.base.StrEnumType for stringified enum columns

Warning

If your ./py -m alembic current says 7ff52ff68bfc, you must downgrade the existing migration first BEFORE applying this PR's commit with a manual stamping afterwards,
and then upgrade again AFTER applying this PR's commit (no longer manual stamping required).

git pull  # checkout the main branch after this PR is merged
git checkout 29e940b659e61410d0007de1cd30691b65929cf7  # the parent commit of this PR's merge commit
./py -m alembic downgrade a5319bfc7d7c
./py -m alembic stamp a5319bfc7d7c
git checkout main
./py -m alembic upgrade head

If you haven't applied the PR commit f5a8bbf from #1892, you may just simply run the upgrade.
(i.e., when your ./py -m alembic current does NOT say 7ff52ff68bfc, just git pull && ./py -m alembic upgrade head)

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Mention to the original issue

- Use varchar(64) type instead of enum for ease of future migrations
- Perform the status_history migration using jsonb functions at once,
  instead of fetching and looping over rows.
@achimnol achimnol added this to the 24.03 milestone Feb 28, 2024
@achimnol achimnol added comp:manager Related to Manager component type:enhance Enhance component, behavior, internals without user-facing features labels Feb 28, 2024
@achimnol achimnol self-assigned this Feb 28, 2024
@github-actions github-actions bot added require:db-migration Automatically set when alembic migrations are added or updated size:L 100~500 LoC labels Feb 28, 2024
@achimnol achimnol added skip:changelog Make the action workflow to skip towncrier check and removed require:db-migration Automatically set when alembic migrations are added or updated size:L 100~500 LoC labels Feb 28, 2024
@achimnol achimnol added this pull request to the merge queue Feb 28, 2024
Merged via the queue into main with commit 1fec5cf Feb 28, 2024
54 of 73 checks passed
@achimnol achimnol deleted the fix/rewrite-vfolder-status-migration branch February 28, 2024 08:05
achimnol added a commit that referenced this pull request Feb 28, 2024
- It is a follow-up to #1892 and #1936.
- Adds `index=True` to the `kernels.role` column to avoid mismatch of DB
  and migrations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component skip:changelog Make the action workflow to skip towncrier check type:enhance Enhance component, behavior, internals without user-facing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant