Opened 2 years ago

Closed 2 years ago

#27832 closed Bug (invalid)

contenttypes migration not following the doc on hints naming

Reported by: Hervé Cauwelier Owned by: Hervé Cauwelier
Component: contrib.contenttypes Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no



The second migration for contenttypes provides a RunPython operation:

As you can see on the highlighted line, the hint is named "model_name".

But the doc suggests the key should be "model":

Not a big issue but I ran into this when writing the "allow_migrate" method of a database router.

Change History (7)

comment:1 Changed 2 years ago by Tim Graham

Component: Migrationscontrib.contenttypes
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

For backwards compatibility at this point, I guess we should add 'model' there instead of renaming 'model_name'.

comment:2 Changed 2 years ago by Hervé Cauwelier

Owner: changed from nobody to Hervé Cauwelier
Status: newassigned

I guess it's a low-hanging fruit for a newcomer like me, assigning it to myself.

comment:3 Changed 2 years ago by Hervé Cauwelier

Has patch: set

comment:4 Changed 2 years ago by Tim Graham

Severity: Release blockerNormal

Now that I see that migration was added in Django 1.8, I'm not sure if there's much value in fixing this. 1.8/1.9 only receive security fixes at this point. Does the fix have some value if it's changed in later versions of Django?

Separately, we might consider a strategy for squashing contrib app migrations so we can eventually remove obsolete migrations like that one.

comment:5 Changed 2 years ago by Hervé Cauwelier

I always assumed you meant Django 1.10 and later. Django 1.8 and 1.9 happily lived with this typo. :-)

So we could just leave both keys without a deprecation path and that migration will naturally disappear when migrations are squashed?

As for the added value of this fix, it would avoid me to write a special case in my database router when "model_name" is "contenttype".

comment:6 Changed 2 years ago by Hervé Cauwelier

Oops! I've added some print()s in my code to discover it was just a coincidence if the router didn't choke on KeyError. With the patch, it's trigerring another code path or something, let me sort this out first.

comment:7 Changed 2 years ago by Hervé Cauwelier

Resolution: invalid
Status: assignedclosed

OK I missed the point when hints contains "model_name" and it's extracted and passed as an argument to allow_migration().

So yes indeed it was triggering the code path in my router when "model_name" is defined.

This patch in django just prevented passing a "model_name" argument in the first place, but any name would do, even "foobar".

Closing this ticket, thank you for your time.

Note: See TracTickets for help on using tickets.
Back to Top