urbanplatform / django-keycloak-auth

Middleware to allow authorization using Keycloak and Django for django-rest-framework (DRF). This package should only be used in projects starting from scratch, since it overrides the users' management.
MIT License
32 stars 14 forks source link

Fix migration issues with django native "squashmigrations" command #34

Closed diogosilva30 closed 1 year ago

diogosilva30 commented 1 year ago

@moritz89 Please review this new PR. I have looked for a more native solution to the migration problem, and have found the django squashmigrations command. This command creates a new migration that replaces existing ones. This safely keeps all existing migrations for existing production systems, and creates a new 0001_squashed_0005_auto_20211231_1702.py for new deployments. With this change we encountered no problems in a existing system and new systems. Once all existing production systems have applied this migrations, the old ones can be safely deleted if needed. This solution was discussed and approved with @rjfv @ubi-rvitorino

Please review Django Documentation on this command: https://docs.djangoproject.com/en/4.1/topics/migrations/#squashing-migrations

uw-rvitorino commented 1 year ago

I'll approve and merge this one @diogosilva30 (cc @moritz89 ), but we need to create an issue for the future...

image

... to remind us to convert the squashed migration into a normal one and leave the Django gods happy with us. 😇

According to the official documentation:

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:

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

Thanks for this!

uw-rvitorino commented 1 year ago

@diogosilva30 before approving, I'd strongly recommend giving a proper name to the migration file, instead of the autogenerated one (since it will not be useful for humans in the future):

Use the squashmigrations --squashed-name option if you want to set the name of the squashed migration rather than use an autogenerated one.

uw-rvitorino commented 1 year ago

@moritz89 what are your thoughts on merging this before #33 ? I don't envision many conflicts, although your MR requires renaming a few migrations (as far as I recall)

diogosilva30 commented 1 year ago

@uw-rvitorino I have renamed the migration to "redo_migrations_0001to0005". The resulting file is 0001_redo_migrations_0001to0005.py. Is this name ok?

moritz89 commented 1 year ago

@moritz89 what are your thoughts on merging this before #33 ? I don't envision many conflicts, although your MR requires renaming a few migrations (as far as I recall)

Sure lets merge this MR first. #33 basically just moves them, so its just a small fix.