Include alembic migrations - #1584
Open
sebdiem wants to merge 3 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds optional Alembic migration support to Procrastinate by introducing 38 Alembic revision files ( ChangesAlembic migration support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The 38 revision files are structurally identical (low per-file reasoning cost), but the first revision's pattern, the test helpers, and the schema accessor warrant separate inspection. The bulk of the diff is highly repetitive. Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
sorry for the double submission I inadvertently closed the previous one |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/migration/test_migration.py (1)
217-232: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAST-based revision extraction is correct but fragile to naming changes.
The sorted glob assumes lexicographic order matches revision order (which holds for
procrastinate_NNNNnaming). The AST parsing correctly extracts therevisionconstant. This approach works reliably for the current naming convention.Consider adding a brief inline comment explaining why sorted-last gives the head revision, to help future maintainers:
# Files are named procrastinate_NNNN_*.py; sorted-last is the head procrastinate_head = sorted(...)[-1]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/migration/test_migration.py` around lines 217 - 232, Add a brief inline comment before the line with `procrastinate_head = sorted(...)[-1]` that explains why taking the sorted-last item gives the head revision. The comment should note that files are named with the pattern `procrastinate_NNNN_*.py` where numeric ordering matches lexicographic ordering, making the sorted-last approach reliable for identifying the head revision file. This helps future maintainers understand the assumption behind this code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/migration/test_migration.py`:
- Around line 217-232: Add a brief inline comment before the line with
`procrastinate_head = sorted(...)[-1]` that explains why taking the sorted-last
item gives the head revision. The comment should note that files are named with
the pattern `procrastinate_NNNN_*.py` where numeric ordering matches
lexicographic ordering, making the sorted-last approach reliable for identifying
the head revision file. This helps future maintainers understand the assumption
behind this code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96fb5fd7-688d-4a94-9662-5168e09fc0e4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.pre-commit-config.yamldocs/howto/production/migrations.mdprocrastinate/alembic/__init__.pyprocrastinate/alembic/versions/__init__.pyprocrastinate/alembic/versions/procrastinate_0000_initial.pyprocrastinate/alembic/versions/procrastinate_0001_drop_started_at_column.pyprocrastinate/alembic/versions/procrastinate_0002_drop_started_at_column.pyprocrastinate/alembic/versions/procrastinate_0003_drop_procrastinate_version_table.pyprocrastinate/alembic/versions/procrastinate_0004_fix_procrastinate_fetch_job.pyprocrastinate/alembic/versions/procrastinate_0005_fix_trigger_status_events_insert.pyprocrastinate/alembic/versions/procrastinate_0006_add_queueing_lock_column.pyprocrastinate/alembic/versions/procrastinate_0007_close_fetch_job_race_condition.pyprocrastinate/alembic/versions/procrastinate_0008_add_defer_job_function.pyprocrastinate/alembic/versions/procrastinate_0009_add_procrastinate_periodic_defers.pyprocrastinate/alembic/versions/procrastinate_0010_add_foreign_key_index.pyprocrastinate/alembic/versions/procrastinate_0011_add_locks_to_periodic_defer.pyprocrastinate/alembic/versions/procrastinate_0012_fix_procrastinate_defer_periodic_job.pyprocrastinate/alembic/versions/procrastinate_0013_add_finish_job_and_retry_job_functions.pyprocrastinate/alembic/versions/procrastinate_0014_add_trigger_on_job_deletion.pyprocrastinate/alembic/versions/procrastinate_0015_delete_finished_jobs.pyprocrastinate/alembic/versions/procrastinate_0016_add_checks_to_finish_job.pyprocrastinate/alembic/versions/procrastinate_0017_add_checks_to_retry_job.pyprocrastinate/alembic/versions/procrastinate_0018_fix_finish_job_compat_issue.pyprocrastinate/alembic/versions/procrastinate_0019_add_index_on_procrastinate_jobs.pyprocrastinate/alembic/versions/procrastinate_0020_add_kwargs_to_defer_periodic_job.pyprocrastinate/alembic/versions/procrastinate_0021_null_locks_excluded.pyprocrastinate/alembic/versions/procrastinate_0022_remove_old_finish_job_function.pyprocrastinate/alembic/versions/procrastinate_0023_job_id_bigint.pyprocrastinate/alembic/versions/procrastinate_0024_add_job_priority.pyprocrastinate/alembic/versions/procrastinate_0025_add_periodic_job_priority.pyprocrastinate/alembic/versions/procrastinate_0026_add_cancel_states.pyprocrastinate/alembic/versions/procrastinate_0027_add_additional_params_to_retry_job.pyprocrastinate/alembic/versions/procrastinate_0028_add_indexes_for_fetch_job.pyprocrastinate/alembic/versions/procrastinate_0029_pre_cancel_notification.pyprocrastinate/alembic/versions/procrastinate_0030_post_cancel_notification.pyprocrastinate/alembic/versions/procrastinate_0031_pre_add_heartbeat.pyprocrastinate/alembic/versions/procrastinate_0032_post_add_heartbeat.pyprocrastinate/alembic/versions/procrastinate_0033_pre_batch_defer_jobs.pyprocrastinate/alembic/versions/procrastinate_0034_post_batch_defer_jobs.pyprocrastinate/alembic/versions/procrastinate_0035_pre_priority_lock_fetch_job.pyprocrastinate/alembic/versions/procrastinate_0036_pre_add_retry_failed_job_procedure.pyprocrastinate/alembic/versions/procrastinate_0037_post_add_retry_failed_job_procedure.pyprocrastinate/schema.pypyproject.tomltests/conftest.pytests/migration/test_migration.pytests/unit/test_schema.py
✅ Files skipped from review due to trivial changes (12)
- procrastinate/alembic/versions/procrastinate_0011_add_locks_to_periodic_defer.py
- procrastinate/alembic/versions/procrastinate_0035_pre_priority_lock_fetch_job.py
- procrastinate/alembic/versions/procrastinate_0028_add_indexes_for_fetch_job.py
- procrastinate/alembic/versions/procrastinate_0007_close_fetch_job_race_condition.py
- procrastinate/alembic/versions/procrastinate_0008_add_defer_job_function.py
- procrastinate/alembic/init.py
- procrastinate/alembic/versions/init.py
- procrastinate/alembic/versions/procrastinate_0002_drop_started_at_column.py
- tests/conftest.py
- procrastinate/alembic/versions/procrastinate_0017_add_checks_to_retry_job.py
- procrastinate/alembic/versions/procrastinate_0019_add_index_on_procrastinate_jobs.py
- docs/howto/production/migrations.md
sebdiem
changed the title
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
1 participant
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.
Procrastinate ships optional Alembic revisions that wrap the same raw SQL
migration scripts already used by
procrastinate schema --apply.Independent branches via
branch_labelsEach procrastinate revision module declares a
revision, adown_revisionpointing to the previous procrastinate revision, and — critically —
branch_labelson the root revision:This means procratinate's revision chain lives on a separate Alembic
branch named
procrastinate. A user's existing revision chain (whichalso starts from
down_revision = Nonebut has no branch label) coexistspeacefully with procrastinate's branch — Alembic treats them as two
independent trees and tracks both heads in
alembic_version.No merge is required. If a user's own schema change must run after a
specific procrastinate revision, they set
down_revisionto thatprocrastinate revision id in their own migration, exactly as they would for
any other dependency.
Resolving the versions directory
Users add procrastinate's versions to their Alembic
version_locations:The
procrastinate:alembic/versionsentry is resolved by Alembic from theinstalled Python package, so it works regardless of where the virtualenv or
site-packages directory lives.
Minimum Alembic version
The library uses only
op.get_context().autocommit_block(),op.execute(), andversion_locationsall available since very early alembic.Disclaimer
Closes #1040
Successful PR Checklist:
PR label(s):
Summary by CodeRabbit
Release Notes
New Features
version_locations.Documentation