Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#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)

comment:1 Changed 3 years ago by Johannes Maron

Cc: Johannes Maron added
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

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.

comment:2 in reply to:  1 Changed 3 years ago by amureki

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 Changed 3 years ago by amureki

Has patch: set
Needs tests: set
Owner: changed from nobody to amureki
Status: newassigned

comment:4 Changed 3 years ago by Johannes Maron

Patch needs improvement: set

comment:5 Changed 3 years ago by amureki

Patch needs improvement: unset

comment:6 Changed 3 years ago by Johannes Maron

Needs tests: unset
Triage Stage: AcceptedReady for checkin

comment:7 Changed 3 years ago by Mariusz Felisiak

Severity: NormalRelease blocker

comment:8 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In a24fed39:

Fixed #32733 -- Skipped system check for specifying type of auto-created primary keys on abstract models.

Regression in b5e12d490af3debca8c55ab3c1698189fdedbbdb.

comment:9 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 349bb58b:

[3.2.x] Fixed #32733 -- Skipped system check for specifying type of auto-created primary keys on abstract models.

Regression in b5e12d490af3debca8c55ab3c1698189fdedbbdb.

Backport of a24fed399ced6be2e9dce4cf28db00c3ee21a21c from main

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