[Django] #32285: AppConfig: app_label with dot should raise exception

33 views
Skip to first unread message

Django

unread,
Dec 20, 2020, 12:54:21 PM12/20/20
to django-...@googlegroups.com
#32285: AppConfig: app_label with dot should raise exception
------------------------------------------------+------------------------
Reporter: Federico Capoano | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 3.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+------------------------
Follow up to [https://groups.google.com/g/django-
developers/c/3cbMviLrSzI/m/KEXlMDD3AAAJ mailing list discussion].

Setting the `app_label` of an `AppConfig` object is allowed and fails
later on during migrations (`./manage.py migrate`) with `(too many values
to unpack)`.
It took me a couple of minutes to figure out what was wrong, but it may
not be so obvious to an inexperienced user so it may be better to raise a
clearer exception.

Maybe we can use system checks and fail earlier if such a problem is
detected?

--
Ticket URL: <https://code.djangoproject.com/ticket/32285>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 21, 2020, 12:57:23 AM12/21/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------

Reporter: Federico Capoano | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution: needsinfo

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => needsinfo
* type: Bug => Cleanup/optimization
* component: Core (System checks) => Core (Other)


Comment:

Raising `ImproperlyConfigured` should be enough, however everything works
for me with dotted label, e.g. `ticket_32285.test_one`. Can you provide a
sample project?

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:1>

Django

unread,
Dec 21, 2020, 2:31:41 AM12/21/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
--------------------------------------+------------------------------------

Reporter: Federico Capoano | Owner: nobody
Type: Cleanup/optimization | Status: new

Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):

* status: closed => new
* resolution: needsinfo =>
* stage: Unreviewed => Accepted


Comment:

It looks that it will be good to raise an error to avoid confusions (see
#32287), even if I cannot reproduce this crash.

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:2>

Django

unread,
Dec 21, 2020, 8:45:32 AM12/21/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned

Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

* owner: nobody => Hasan Ramezani
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:3>

Django

unread,
Dec 21, 2020, 11:15:59 AM12/21/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Federico Capoano):

Have you already tried to create a new django app, put a dot in the
app_label field of the AppConfig, ensure the AppConfig is mentioned in the
__init__.py, then create a model and run ./manage.py makemigrations?

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:4>

Django

unread,
Dec 21, 2020, 12:18:32 PM12/21/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Replying to [comment:4 Federico Capoano]:


> Have you already tried to create a new django app, put a dot in the
app_label field of the AppConfig, ensure the AppConfig is mentioned in the
__init__.py, then create a model and run ./manage.py makemigrations?

Yes. Can you share a stacktrace? I'm not even sure in which line or file
you encountered this issue 🤔

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:5>

Django

unread,
Dec 21, 2020, 12:19:36 PM12/21/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1
* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/13796 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:6>

Django

unread,
Dec 21, 2020, 4:06:32 PM12/21/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

* needs_better_patch: 1 => 0


--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:7>

Django

unread,
Dec 22, 2020, 2:56:10 AM12/22/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:8>

Django

unread,
Dec 22, 2020, 7:28:01 AM12/22/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: closed

Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"8b2a30f6f16cb1f3538847954030d69da005bc7f" 8b2a30f]:
{{{
#!CommitTicketReference repository=""
revision="8b2a30f6f16cb1f3538847954030d69da005bc7f"
Fixed #32285 -- Raised ImproperlyConfigured when AppConfig.label is not a
valid Python identifier.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:10>

Django

unread,
Dec 22, 2020, 7:28:01 AM12/22/20
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: assigned

Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"110001d0bbbabe2a5b57b14a59bd0e4b71bf2712" 110001d]:
{{{
#!CommitTicketReference repository=""
revision="110001d0bbbabe2a5b57b14a59bd0e4b71bf2712"
Refs #32285 -- Made AppConfigStub do not call super().__init__().

Calling super().__init__() is unnecessary and enforces the use of
various workarounds.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:9>

Django

unread,
Jul 2, 2021, 1:26:55 PM7/2/21
to django-...@googlegroups.com
#32285: AppConfig.label should raise an exception when it's not a valid Python
identifier.
-------------------------------------+-------------------------------------
Reporter: Federico Capoano | Owner: Hasan
Type: | Ramezani
Cleanup/optimization | Status: closed

Component: Core (Other) | Version: 3.1
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Weber):

I was surprised by this error when trying to upgrade a large codebase to
3.2. This codebase's convention is to use app labels with dashes, which I
suspect still works fine (unlike labels with periods).

Is there a recommended migration path for projects like these that were
using an invalid label unknowingly? As far as I can tell, the only option
is to change the label, which looks involved: it affects things like old
migrations, db rows like content types, and maybe even table names.

If there's no simple migration path, what about changing this from an
exception to a warning? That'd still accomplish the goal of informing
users that something may break and allows cases like mine to avoid quite a
bit of pain.

--
Ticket URL: <https://code.djangoproject.com/ticket/32285#comment:11>

Reply all
Reply to author
Forward
0 new messages