#32733 closed Cleanup/optimization (fixed)
DEFAULT_AUTO_FIELD feature leads `AbstractModel.check()` to fail with AttributeError
Reported by: | amureki | Owned by: | amureki |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.2 |
Severity: | Release blocker | Keywords: | |
Cc: | Johannes Maron | 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
Here is a simple code that shows different behaviour between 3.1 and 3.2:
from django.db import models class MyAbstractModel(models.Model): class Meta: abstract = True # behaviour in 3.1.10: MyAbstractModel.check() # returns [] # behaviour in 3.2: MyAbstractModel.check() # raises AttributeError: 'NoneType' object has no attribute 'auto_created' in `_check_default_pk`
It seems to me (but I might be wrong here) that this was not an intentional change (as the feature is all about default autofield), but rather a bug.
However, feel free to correct me!
Best,
Rust
Change History (9)
follow-up: 2 comment:1 by , 3 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
comment:2 by , 3 years ago
Replying to Johannes Maron:
It is arguable, whether if the checks need to work on an abstract class. I also noticed that Django will raise an error since 3.2 if you try to instantiate an abstract model.
Though, there is no place where checks are called on abstract models inside an application, this does make unit testing harder.
With that in mind, maybe this is best classified as an optimization not a bug, since no public API is broken, not does this affect the application itself. However, since Django is a developer-friendly framework and comes with an excellent test suite, I believe an improvement would be welcome by many.
Thanks, Johannes!
I made a draft PR with blunt solution - freshly introduced auto-field check would not run on abstract models.
I am more than happy to get better ideas or any other feedback!
comment:3 by , 3 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 3 years ago
Patch needs improvement: | set |
---|
comment:5 by , 3 years ago
Patch needs improvement: | unset |
---|
comment:6 by , 3 years ago
Needs tests: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:7 by , 3 years ago
Severity: | Normal → Release blocker |
---|
It is arguable, whether if the checks need to work on an abstract class. I also noticed that Django will raise an error since 3.2 if you try to instantiate an abstract model.
Though, there is no place where checks are called on abstract models inside an application, this does make unit testing harder.
With that in mind, maybe this is best classified as an optimization not a bug, since no public API is broken, not does this affect the application itself. However, since Django is a developer-friendly framework and comes with an excellent test suite, I believe an improvement would be welcome by many.