Any plans to release v1.3.7 with support for Django 5.1?

[scribejourdan]

I'm trying to upgrade Django to 5.1 but I'm unable because this package triggers an indexed_together exception on ver sion 1.3.6.

Additionally, on version 1.3.7b1, poetry throws this error:

Because django-easy-audit (1.3.7b1) depends on django (>=4.2,<5.0)
 and no versions of django-easy-audit match >1.3.7b1,<1.4.0, django-easy-audit (>=1.3.7b1,<1.4.0) requires django (>=4.2,<5.0).
So, because non-package-mode depends on both django-easy-audit (~1.3.7b1) and Django (~5.1), version solving failed.

[jheld]

Great question. I've just released 1.3.7.b3 (1.3.7.b2 includes a version syntax error) which intends to broaden the django support from >=4.2, <6.0 and I am hoping helps get around the issue you were seeing.

[djch]

FWIW, I'm still getting the index_together error after installing the 1.3.7b3 release.

No other instances of index_together in my codebase.

[jheld]

Which django version are you on now?

I might not be able to check into this more short term but perhaps @mschoettle has any ideas?

[djch]

On Django 5.1.

If I downgrade to back 5.0.8, there's no problem (same as v1.3.6).

[mschoettle]

@djch can you please provide the full stack trace of the error you are getting?

[scribejourdan]

@mschoettle @jheld I'm having no issues anymore, thanks!

Despite of that, I'd really like to know if there's any projection on when 1.3.7 will be released i.e. not beta. It's a little hard to pitch an update based on beta versions.

[scribejourdan]

Well, actually, I just faced the same issue @djch experienced.

Tracked it down to the migration 0004.

I saw a squashed 0004 migration there which should replace the original 0004, but Django still loads that migration, it seems. I placed a debugger there to make sure, it got hit.

I'm being able to run everything that don't trigger that migration, so I can start the server and run manage commands. This is happening solely when I run tests, I assume because the database gets built from scratch, so it goes through all migrations.

If you kept the original migrations concerned they'd be needed, you don't need to worry. As long as the squash migration reaches the exact same state as the other combined and they're referenced at the replaces property, it'll be alright.

[djch]

Sounds like @scribejourdan probably has the right of the situation.

That being said, I just updated Django to 5.1 again to confirm and now I don't get the error... 🥲

I can say I'm pretty sure I was running ./manage.py test to trigger it, though

[PaarthShah]

@scribejourdan I filed the issue as #303 and introduced PR #304 to fix it. Verified myself that it works.

It's caused by already having partially migrated, but not past 0019. Without that, the unsquashed migrations still have to run until 0019 before proceeding, and that causes 0004 to be loaded, which fails.

[mschoettle]

I might not be able to check into this more short term but perhaps @mschoettle has any ideas?

I had a look and thought about this a bit and figured the following out:

We squashed the migration(s) with the (at the time) deprecated index_together in #285. The ideal workflow following that would have been to have users of django-easy-audit upgrade to a new version with the squashed migration that they then migrate to. On a version before Django 5.1. Now that Django 5.1 is out, where index_together was removed completely this causes this error because the old migration is still there and "in use".

Are people on here experiencing this issue using Django 5.1 already in production or is this when testing the upgrade to 5.1?

If it is the latter, a solution would be to first upgrade django-easy-audit and apply the migrations. Once that is done upgrade to Django 5.1. I understand that there is reluctance to upgrade to a beta version. It would be good to release a proper version.

/cc @jheld

[scribejourdan]

Production flow should actually work fine.

The problem I'm having is with running tests on CI. Django creates a database from scratch and loads those migration files, even if not actually running them, and fails with that error.

[mschoettle]

I cannot reproduce this. Tried it on our project with Django==5.1 and django-easy-audit==1.3.7b3 and pytest runs without an error related to migrations in CI.

I also tried a minimal reproduction:

docker run --rm -it python:3.12-alpine sh
mkdir app
cd app
pip install Django==5.1
pip install django-easy-audit==1.3.7b3
django-admin startproject test .
# add easyaudit to INSTALLED_APPS in test/settings.py
python manage.py showmigrations
python manage.py migrate

Output of showmigrations

``` /app # python manage.py showmigrations admin [ ] 0001_initial [ ] 0002_logentry_remove_auto_add [ ] 0003_logentry_add_action_flag_choices auth [ ] 0001_initial [ ] 0002_alter_permission_name_max_length [ ] 0003_alter_user_email_max_length [ ] 0004_alter_user_username_opts [ ] 0005_alter_user_last_login_null [ ] 0006_require_contenttypes_0002 [ ] 0007_alter_validators_add_error_messages [ ] 0008_alter_user_username_max_length [ ] 0009_alter_user_last_name_max_length [ ] 0010_alter_group_name_max_length [ ] 0011_update_proxy_permissions [ ] 0012_alter_user_first_name_max_length contenttypes [ ] 0001_initial [ ] 0002_remove_content_type_name easyaudit [ ] 0001_initial [ ] 0002_auto_20170125_0759 [ ] 0003_auto_20170228_1505 [ ] 0004_auto_20170620_1354_squashed_0019_alter_crudevent_changed_fields_and_more (16 squashed migrations) sessions [ ] 0001_initial ```

Output of migrate

``` /app # python manage.py migrate Operations to perform: Apply all migrations: admin, auth, contenttypes, easyaudit, sessions Running migrations: Applying contenttypes.0001_initial... OK Applying auth.0001_initial... OK Applying admin.0001_initial... OK Applying admin.0002_logentry_remove_auto_add... OK Applying admin.0003_logentry_add_action_flag_choices... OK Applying contenttypes.0002_remove_content_type_name... OK Applying auth.0002_alter_permission_name_max_length... OK Applying auth.0003_alter_user_email_max_length... OK Applying auth.0004_alter_user_username_opts... OK Applying auth.0005_alter_user_last_login_null... OK Applying auth.0006_require_contenttypes_0002... OK Applying auth.0007_alter_validators_add_error_messages... OK Applying auth.0008_alter_user_username_max_length... OK Applying auth.0009_alter_user_last_name_max_length... OK Applying auth.0010_alter_group_name_max_length... OK Applying auth.0011_update_proxy_permissions... OK Applying auth.0012_alter_user_first_name_max_length... OK Applying easyaudit.0001_initial... OK Applying easyaudit.0002_auto_20170125_0759... OK Applying easyaudit.0003_auto_20170228_1505... OK Applying easyaudit.0004_auto_20170620_1354_squashed_0019_alter_crudevent_changed_fields_and_more... OK Applying sessions.0001_initial... OK ```

[scribejourdan]

Interesting. I'll have it tested again tomorrow and bring a stack trace of how Django reaches that code.

Thanks!

[jheld]

Happy to make the proper release soon-ish (if we determine the risk is quite low or if we determine a patch). Given the issue we're seeing though, I'm kind of happy that it is not the next official release yet!

[scribejourdan]

Didn't have time to trigger the error today, day was pretty packed.

The fix is just to remove the squashed migrations, though. There's no need to keep them around once there's a squash migration to replace them.

Django warns to keep them around until it's deployed in all environments, but that doesn't take Git into account. We can bring them back if we need.

[jheld]

@scribejourdan I filed the issue as #303 and introduced PR #304 to fix it. Verified myself that it works.

It's caused by already having partially migrated, but not past 0019. Without that, the unsquashed migrations still have to run until 0019 before proceeding, and that causes 0004 to be loaded, which fails.

@scribejourdan @mschoettle what was the consensus on accepting this PR?

[scribejourdan]

The operation that adds the index needs to be removed given that the name was added during index creation.

IIRC, that's set at migration 0018.

[scribejourdan]

I got the Django version like import django; dj_vesion=django.__version__.

You can see I have the squashed migration there but the error is still happening.

Now here you can see how I have the squashed migration but when I run a test, it still loads the old migration file, even if not ultimately running it.

The solution is to either fix the original migration (and remove the operation from 0018) or delete the squashed migrations, which is fine to do and even considered a good cleanup practice.

I'll create a PR removing the squashed migrations. Please, take a special look if you can. In our context, it's preventing from upgrading to Django 5.1 which introduces database pools and we really need that to move forward with a bunch of plans that are currently stale.

[scribejourdan]

@jheld Here it is. This will eliminate backwards compatibility issues for good: https://github.com/soynatan/django-easy-audit/pull/305

[mschoettle]

@scribejourdan Which Django version are you using?

And how do you execute the tests?

Please also show the output of python manage.py showmigrations.

[scribejourdan]

The Django version is 5.1. I run tests via Django's test management command.

The showmigrations actually showed me the issue:

easyaudit
 [X] 0001_initial
 [X] 0002_auto_20170125_0759
 [X] 0003_auto_20170228_1505
 [X] 0004_auto_20170620_1354
 [X] 0005_auto_20170713_1155
 [X] 0006_auto_20171018_1242
 [X] 0007_auto_20180105_0838
 [X] 0008_auto_20180220_1908
 [X] 0009_auto_20180314_2225
 [X] 0010_repr_text
 [X] 0011_auto_20181101_1339
 [X] 0012_auto_20181018_0012
 [X] 0013_auto_20190723_0126
 [X] 0014_auto_20200513_0008
 [X] 0015_auto_20201019_1217
 [X] 0016_alter_crudevent_event_type
 [ ] 0017_alter_requestevent_datetime
 [ ] 0018_rename_crudevent_object_id_content_type_index
 [ ] 0019_alter_crudevent_changed_fields_and_more

It won't be able to partially apply the squash migration. Wondering what to do with that.

[mschoettle]

And the installed easy audit version is?

Try migrate easyaudit, then show the output of showmigrations again.

[scribejourdan]

The version is the latest, 1.3.7b3.

Recreated the database from scratch, now it works fine, and the issue has vanished from CI, probably due to the squash migration.

I was wondering if this would be an issue in production environments, but here we're constantly running the migrate command whenever we deploy, so that won't be a problem.

[mschoettle]

Great!

It would only be a problem if the migrations are not applied to the latest one(s) (i.e., the migration that squashes the old ones).

@jheld My recommendation is to release a version that does not work with Django 5.1 first (so based off of 1.3.7b2) to force people to upgrade the migrations and be on the squashed one. Then we can release a new version that supports Django 5.1 and could even remove all the old migrations that were squashed. This is as per the recommendation by Django, see my comment in detail here: https://github.com/soynatan/django-easy-audit/pull/285#issuecomment-2038548945

[PaarthShah]

I'm heavily against any solution where upgrading immediately to the latest django 5.1+ and django-easy-audit is not guaranteed to work.

[jourdanrodrigues]

I just tested and can confirm that RenameIndex does not crash if the new index name is the same as the existing one.

The PR from @PaarthShah (#304) should be safe to be merged.

A caveat I can think, though, is that we don't need the squashed migration in that scenario given that I don't know how it behaves on partially applied migrations.

cc @jheld @mschoettle

[PaarthShah]

@jourdanrodrigues I can answer that:

If a system has partially-applied migrations, (aka is in the middle of what was squashed), then it is forced to use the operations of the unsquashed migrations (but afterwards, will show that the squashed migration has run).

Squashing a migration is asserting that every action in it is equivalent to running all of the individual ones, on systems that started from scratch.

Squashed migrations can thus "elide" (skip) data migrations (including RunPython migrations marked as elidable), or migrations that basically boil down to "create a column and immediately delete it"

Tldr: as long as the unsquashed migrations remain, it's safe. But we CANNOT remove the squashed one either, as anyone who has installed the beta (or from my branch) would then be screwed too).

[jourdanrodrigues]

@PaarthShah Thanks!

[mschoettle]

@PaarthShah

I'm heavily against any solution where upgrading immediately to the latest django 5.1+ and django-easy-audit is not guaranteed to work.

I agree. And the intention of my comment was to outline a process that prevents that. Is there any scenario that you see where this is not guaranteed to work?

[PaarthShah]

@mschoettle

Regarding this bit below: there should not be a 2-step upgrade process. There should only be a single new version, that supports django 5.1 (and has both the squashed and unquashed migrations). There's been no need to update to the latest for any other reason other than django 5.1 support.

A fast-follow version can make no changes other than removing the squashed migration, and can be upgraded to freely.

New features, if they come, will come after this, and inherently depend on people being on 5.1

My recommendation is to release a version that does not work with Django 5.1 first (so based off of 1.3.7b2) to force people to upgrade the migrations and be on the squashed one. Then we can release a new version that supports Django 5.1

[mschoettle]

The process I outlined is what the Django documentation on squashing migrations recommends:

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, ensure your users upgrade releases in order without skipping any), and then remove the old files, commit and do a second release.

Unfortunately this did not happen before 5.1 was released.

New features, if they come, will come after this, and inherently depend on people being on 5.1

No. This package still supports Django versions before 5.1 (>= 4.2).

[scribejourdan]

Hey @jheld sorry to bother, but do you have any idea on when we'd be able to move forward with the upgrade?

[jheld]

Just waiting on PR #304 . If you are open to forking their change, rebase and resolve conflicts, please do. I've just reached out to them on the PR though so maybe give them a few days, and if see not by mid-week.

[PaarthShah]

I'll be able to get to it by tomorrow, just will do a rebase

[PaarthShah]

@jheld done