#33952 closed Bug (fixed)
Too aggressive pk control in create_reverse_many_to_one_manager
| Reported by: | Claude Paroz | Owned by: | David Wobrock |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 4.1 |
| Severity: | Release blocker | Keywords: | |
| Cc: | David Wobrock | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
In the context of #19580, Django now requires an instance pk to even instanciate a related manager [7ba6ebe9149a].
Now I have a use case where I need to introspect the model used by a related manager (MyModel().related_set.model) and Django 4.1 refuses that with ValueError: 'MyModel' instance needs to have a primary key value before this relationship can be used.
My opinion is that is is too aggressive of a check and would suggest to let the __init__ succeed even if the instance has no pk. Other calls to _check_fk_val in the class seems sufficient to me to safeguard against shooting in the foot.
Change History (9)
comment:1 follow-up: 2 Changed 10 months ago by
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 Changed 10 months ago by
Replying to Simon Charette:
...
In the mean time I think you should be able to use
MyModel._meta.get_field("related_set")for your introspection needs.
Thanks for the tip! I just needed to remove the _set as it's not part of the ManyToOneRel field name in _meta.
comment:3 Changed 10 months ago by
| Cc: | David Wobrock added |
|---|---|
| Has patch: | set |
| Owner: | changed from nobody to David Wobrock |
| Status: | new → assigned |
I provided a small PR with the changes discussed by Simon.
There a few behaviour changes, tell me what you think! :)
comment:4 Changed 10 months ago by
| Severity: | Normal → Release blocker |
|---|
Regression in 7ba6ebe9149ae38257d70100e8bfbfd0da189862.
comment:5 Changed 10 months ago by
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:9 Changed 8 months ago by
Replying to Claude Paroz:
Many thanks David for the fix!
Are we sure this is fixed as I still get these errors in all 4.1 versions? I posted an example on another ticket, #33984, which I thought was more related to the issue. you can find that report here
I think this worth doing, having
get_querysetperformself._check_fk_val()should catch the currently tested.all()case as well.In the mean time I think you should be able to use
MyModel._meta.get_field("related_set")for your introspection needs.