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

Refactor and add backward compatibility #59

Closed
wants to merge 1 commit into from
Closed

Conversation

ryanhiebert
Copy link
Owner

@ryanhiebert ryanhiebert commented Jul 20, 2024

Fix #54
Fix #38

The refactor is significant. It needed to reorder operations to allow for calculating safety configuration and delays, but along the way cleaned up naming, the default configuration, and the categories calculated and output to the user.

Rename the internal enums and enum items. Call the enum that represents the run mode of "strict", "nonstrict", or "disabled" Mode, with PEP8 compliant names of STRICT, NONSTRICT, and DISABLED. Call the enum that represents the when each migration is allowed to run When, with names of BEFORE_DEPLOY, AFTER_DEPLOY, and ALWAYS.

Factor out the loops that walk the dependency tree to categorize the migrations into ready, delayed, and blocked. Remove the category of "protected" migrations and combine it with delayed.

Refactor the model manager into a model queryset, and separate filtering the queryset from resolving the final data type.

Only write and read SafeMigration records for migrations that are after_deploy and have a delay set.

Only write SafeMigration records for migrations that are delayed, not blocked or ready, to preserve appropriate semantics for nonstrict mode.

Special-case Safe.before_deploy, Safe.after_deploy, and Safe.always to be allowed as callables for backward compatiblity.

Categorize Safe.after_deploy and Safe.always migrations as blocked if they depend on other blocked migrations. The earliest blocked migrations must still be a Safe.before_deploy migration that depends on a Safe.after_deploy migration.

Change the default safe marking from Safe.after_deploy to Safe.always, to be more reasonable for working with third-party apps that do not use django-safemigrate.

Copy link

codecov bot commented Jul 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7798942) to head (0d72346).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #59   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          188       195    +7     
  Branches        43        49    +6     
=========================================
+ Hits           188       195    +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ryanhiebert ryanhiebert changed the title DO NOT MERGE: A combined refactor for testing Refactor and add backward compatibility Jul 21, 2024
@ryanhiebert ryanhiebert force-pushed the combo-refactor branch 5 times, most recently from 81b0218 to 7fd2c49 Compare July 21, 2024 02:16
@ryanhiebert ryanhiebert marked this pull request as ready for review July 21, 2024 02:17
@ryanhiebert ryanhiebert requested review from tim-schilling and removed request for tim-schilling July 21, 2024 02:17
@ryanhiebert ryanhiebert marked this pull request as draft July 21, 2024 19:23
@ryanhiebert
Copy link
Owner Author

I'm making some changes to pull out items that seem to need more discussion, and to skip the changes I made to hide internal methods and state.

@ryanhiebert ryanhiebert force-pushed the combo-refactor branch 5 times, most recently from 9241a06 to ae8cffe Compare July 21, 2024 22:39
The refactor is significant. It needed to reorder operations to
allow for calculating safety configuration and delays, but along
the way cleaned up naming, the default configuration, and the
categories calculated and output to the user.

Rename the internal enums and enum items. Call the enum that
represents the run mode of "strict", "nonstrict", or "disabled"
Mode, with PEP8 compliant names of STRICT, NONSTRICT, and DISABLED.
Call the enum that represents the when each migration is allowed
to run When, with names of BEFORE_DEPLOY, AFTER_DEPLOY, and ALWAYS.

Factor out the loops that walk the dependency tree to categorize
the migrations into ready, delayed, and blocked. Remove the
category of "protected" migrations and combine it with delayed.

Refactor the model manager into a model queryset, and separate
filtering the queryset from resolving the final data type.

Only write and read SafeMigration records for migrations that are
after_deploy and have a delay set.

Only write SafeMigration records for migrations that are delayed,
not blocked or ready, to preserve appropriate semantics for
nonstrict mode.

Special-case Safe.before_deploy, Safe.after_deploy, and Safe.always
to be allowed as callables for backward compatiblity.

Categorize ``Safe.after_deploy`` and ``Safe.always`` migrations as
blocked if they depend on other blocked migrations. The earliest
blocked migrations must still be a ``Safe.before_deploy`` migration
that depends on a ``Safe.after_deploy`` migration.

Change the default safe marking from ``Safe.after_deploy`` to
``Safe.always``, to be more reasonable for working with third-party
apps that do not use django-safemigrate.
@ryanhiebert
Copy link
Owner Author

Just going to open anything from this up in much smaller PRs

@ryanhiebert ryanhiebert deleted the combo-refactor branch July 24, 2024 05:14
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.

Support backwards compatibility with Safe.before_deploy attributes Change the default
1 participant