hmpf / easydmp

MIT License
7 stars 2 forks source link

Clean away old migrations #193

Closed hmpf closed 3 years ago

hmpf commented 3 years ago

It turns out that if you have a replaceable app (in our case: easydmp_auth replaces default auth) and something that depends on that app (in our case: django's admin), you cannot squash all the migrations of the replaceable app. Weird things happen in some cases. The review sites refuse to be migrated because the admin migration in the migration table exists before the squashed migration of the replaceable auth-app.

Instead of figuring out why migrating on localhost works, while migrating in a review doesn't, I have added back in the first migration of our auth-app and made the squash depend on that. The alternative is probably to edit the django-migrations table and that is much harder to do safely, not to mention having to describe it in the UPGRADING-file. Another possibility is to fake all migrations back to zero (resets the table) then fake them all the way forward again.

For any future squashing of our auth app, we need to make sure to squash from migration 2 onward or update the initial migration manually. Blah.

codecov[bot] commented 3 years ago

Codecov Report

Merging #193 (f7f4bdd) into master (29f2db4) will decrease coverage by 0.58%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
- Coverage   56.59%   56.01%   -0.59%     
==========================================
  Files         115       87      -28     
  Lines        6479     6218     -261     
==========================================
- Hits         3667     3483     -184     
+ Misses       2812     2735      -77     
Impacted Files Coverage Δ
...h/migrations/0001_squashed_0003_upstream_change.py 100.00% <ø> (ø)
.../migrations/0001_squashed_0011_section_optional.py 100.00% <ø> (ø)
.../migrations/0001_squashed_0002_convert_to_jsonb.py 100.00% <ø> (ø)
.../migrations/0001_squashed_0002_convert_to_jsonb.py 100.00% <ø> (ø)
...1_squashed_0003_set_default_planinvitation_type.py 100.00% <ø> (ø)
...igrations/0001_squashed_0005_auto_20201016_1539.py 100.00% <ø> (ø)
..._20201111_1411_squashed_0010_delete_plancomment.py 100.00% <ø> (ø)
src/easydmp/lib/convert/answers.py

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 29f2db4...f7f4bdd. Read the comment docs.

hmpf commented 3 years ago

I think the commit-message monster needs a link to the culprit. Will add and force-push.

vbhavdal commented 3 years ago

@hmpf BTW what is the consequence if we forget to "For any future squashing of our auth app, we need to make sure to squash from migration 2", will it fail on the attempted squash itself?

hmpf commented 3 years ago

I'm starting to think the only robust, future-proof solution to this is fake-resetting the migrations.. I'll test a bit more.

hmpf commented 3 years ago

@hmpf BTW what is the consequence if we forget to "For any future squashing of our auth app, we need to make sure to squash from migration 2", will it fail on the attempted squash itself?

No, it will fail when migrating in production :/ No errors while doing the squashing.

The errors I've gotten was on the reviews, which automigrate. I don't get the errors on a fresh copy of the database when I manually migrate, and we do manually migrate in production, but still.

hmpf commented 3 years ago

I have created a new management command "resetmigrationhistory". As long as it is possible to migrate from zero, this should fix a whole host of migration problems. Basically, it empties the django_migrations table so that infelicitous orderings are removed. Doing a fake migration afterwards (migrate --fake --fake-initial) refills the table with a working migration history.

This fixes the problem I see with the automated review sites.