Django-celery-beat: Messed up migration files

[justmobilize]

Hello! Are there plans to create a new version with corrected migrations? I would love to use some of the new features, but until the migrations are stable, I can't.

I would be happy to help if I can get commitment to the release being rolled out quickly.

[tuky]

similarly to here https://github.com/EliotBerriot/django-dynamic-preferences/issues/23 would it be possible to limit the max_length to 191? my current solution to that involves monkey patching CharField.__init__ which is not nice.

[tuky]

would you welcome a PR which makes the max_length configurable like so? https://github.com/python-social-auth/social-app-django/blob/master/social_django/migrations/0003_alter_email_max_length.py

[justmobilize]

@tuky FYI, that's dangerous, since you can't change it after the migration runs. If this was my repo, I would never support that

[tuky]

I object, you can change the setting and run a custom sql migration, which realizes the new value.

[justmobilize]

True, but many users don't understand how to use that and it can cause a lot of issues.

Either way, goal #1 should be to get the current migrations in a good place

[auvipy]

doesn't the 1.4 release fix the migration problems?

[tuky]

i've had no problems applying all the migrations apart from max_length being too high for MySQL with InnoDB and utf8mb4 (all widely recommended configuration options. well except MySQL ;-). So just found https://github.com/celery/django-celery-beat/pull/202 and I'll try to support that one.

[justmobilize]

It depends on what version you had installed. Migration files after a release shouldn't be edited since they are already being used

[auvipy]

@tinylambda

[moseb]

Migrations were sane at 1.4.0 — except maybe two 0006 files both depending on 0005 — but things have gone "sideways" after that on master regarding migrations (…). Before I go into detail let me quote the Django docs how the squashing process was designed to work:

Django's view on squashing migrations

The recommended process is to squash, keeping the old files, commit and release, wait until all systems are upgraded with the new release (or if you’re a third-party project, just ensure your users upgrade releases in order without skipping any), and then remove the old files, commit and do a second release.

Note the your users upgrade releases in order without skipping any part — something that most projects might have a hard time ensuring for (or requesting from) user. So squashing might not even work well for django-celery-beat's case _at all_.

It goes on saying:

Once you’ve squashed your migration, you should then commit it alongside the migrations it replaces and distribute this change to all running instances of your application, making sure that they run migrate to store the change in their database.

You must then transition the squashed migration to a normal migration by:

• [1.] Deleting all the migration files it replaces.
• [2.] Updating all migrations that depend on the deleted migrations to depend on the squashed migration instead.
• [3.] Removing the replaces attribute in the Migration class of the squashed migration (this is how Django tells that it is a squashed migration).

Out of those, (2) has not happened on master yet.

Status quo on master

So after 1.4.0 we are now with this situation on master (speaking of 47c9513e37fbffbba3975dd6717701e10c13d4f6 as of the moment):

• File 0005_add_solarschedule_events_choices_squashed_0009_merge_20181012_1416.py has lost its replaces section, previously pointing to:
  
  
  
  • 0005_add_solarschedule_events_choices.py
  
  
  • 0006_auto_20180210_1226.py
  
  
  • 0006_auto_20180322_0932.py
  
  
  • 0007_auto_20180521_0826.py
  
  
  • 0008_auto_20180914_1922.py
  
  
• Two more regular migrations — 0009 and 0100 — have been added.
• Those five files have been deleted, which makes 0009 depend on now-gone 0006_periodictask_priority.
• Meta-migration 0011 messes with the migration tracking table to forget those five migrations mentioned above.

In short, the approach taken to squashed migrations rather "unconventional" :smiley:

Why is that a problem?:

• Migration 0009 is "hanging in the air" so migration "from nothing" to 0011 is not going to work
• Migrations introduced with v1.1.1 plus the replaces section have been removed, hence anyone coming from v1.1.1, v1.2.0, v1.3.0 will not be able to migrate. @justmobilize seems to also consider this a problem judging from https://github.com/celery/django-celery-beat/pull/244#issuecomment-487696308 .
• Third-party migrations depending on the removed migrations are going to break

The way forward

This is what propose for a way forward:

• Restore the 5 deleted migrations from their latest version at 0c2586b193b9016ece65782bcafdcef9da0b23e4
• Re-add the replaces section (i.e. also revert that part of 0c2586b193b9016ece65782bcafdcef9da0b23e4))
• Drop migration 0011 altogether
• Done.

That would affectivly undo the squashing, fix the breakage, be more compabible and work without the black-magic in current 0011. Do we really need 0011, even?

What do you think?

Thanks for your patience reading until this point :+1:

[liquidpele]

@moseb Good write up. The issue was that whoever squashed the migrations did... something... that made backward migration and forward again fail. This I took the unconventional approach to get all migrations passing, and added tests as well to hopefully keep it from happening again.

Maybe I messed up though! If you can make a PR and all the migration tests I added pass, then I’d accept it since it’s generally better to not have to manually touch the migrations table.

[justmobilize]

@moseb and @liquidpele, I don't think it was the squashing that was the problem. I think it was multiple PR's got merged in that all were taken from the same point and all had 0006* migrations.

I started with a patch in #210 that has everything working, other then migrating from v1.2.0, but I'm not entirely sure without some hacky-ness if they can all 100% work. But I also didn't devote too much time on it yet.

@moseb, since the squash migration hasn't been finalized in a release, I don't feel there is a need to keep it, do you? The only benefit would be the faster install time for people running tests in their projects, at which point there could be a new one made to cover everything once it's all sorted out. The squashed migration can never get to step 3 above, since this project doesn't own all the place it's installed and it would force strange upgrade paths (vX > vY > vZ, not vX > vZ)

If there is a way I can help, I would be happy to.

[liquidpele]

@justmobilize The squashed migration exists in 1.4.0.

https://github.com/celery/django-celery-beat/tree/v1.4.0/django_celery_beat/migrations

Thus, we can't just undo the squash, we have to fix it.

As I said, I'm happy to look at other solutions, just make sure all the new upgrade tests pass! :)

[moseb]

@moseb, since the squash migration hasn't been finalized in a release, I don't feel there is a need to keep it, do you?

I get the idea but since the file was published as part of 1.4.0, I think other projects might depend on the squashed version wince it has a distinct name that can be referenced. Also, keeping the file would allow faster updates for people doing a multi-step update. What do you think?

[moseb]

If you can make a PR and all the migration tests I added pass, then I’d accept it since it’s generally better to not have to manually touch the migrations table.

I'll see what I can do

[justmobilize]

It doesn't hurt to leave it in there. Just clutter in my opinion.

Removing it would leave a ghost entry in the django_migrations table which isn't ideal.

It could also be left in there with no replaces or operations and just do nothing.

[moseb]

Pull request #246 has just been merged. Is anything left to fix this issue?