[Django] #30300: migrate silently ignores apps whose migrations directory doesn't have an __init__.py
Django
__init__.py
-------------------------------------+-------------------------------------
Reporter: Benjy | Owner: nobody
Weinberger |
Type: Bug | Status: new
Component: Database | Version: 2.1
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Background: In python 3 a package with no `__init__.py` is implicitly a
namespace package, so it has no `__file__` attribute.
The migrate command currently checks for existence of a `__file__`
attribute on the `migrations` package. This check was introduced in
https://code.djangoproject.com/ticket/21015, because the `__file__`
attribute was used in migration file discovery.
However, in https://code.djangoproject.com/ticket/23406 migration file
discovery was changed to use `pkgutil. iter_modules ()`, instead of direct
filesystem access. `pkgutil. iter_modules()` uses the package's
`__path__` list, which exists on implicit namespace packages.
As a result, the `__file__` check is no longer needed, and in fact
prevents `migrate` from working on namespace packages (implicit or
otherwise).
Related work: https://code.djangoproject.com/ticket/29091
--
Ticket URL: <https://code.djangoproject.com/ticket/30300>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
__init__.py
-------------------------------------+-------------------------------------
| Weinberger
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Benjy Weinberger
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:1>
Django
__init__.py
-------------------------------------+-------------------------------------
| Weinberger
Type: Bug | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Benjy Weinberger):
Proposed fix in https://github.com/django/django/pull/11141
(branch https://github.com/benjyw/django/tree/ticket_30300)/
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:2>
Django
__init__.py
-------------------------------------+-------------------------------------
| Weinberger
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Bug => New feature
* stage: Unreviewed => Accepted
Comment:
Hi Benjy. Thanks for the report — and the test case in the PR! Unless
something comes up in review, this seems reasonable to me.
I'm going to call it a new feature, since this behaviour will never have
worked I think.
>... (implicit or otherwise)
I don't quite follow the `or otherwise` there?
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:4>
Django
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> Background: In python 3 a package with no `__init__.py` is implicitly a
> namespace package, so it has no `__file__` attribute.
>
> The migrate command currently checks for existence of a `__file__`
> attribute on the `migrations` package. This check was introduced in
> https://code.djangoproject.com/ticket/21015, because the `__file__`
> attribute was used in migration file discovery.
>
> However, in https://code.djangoproject.com/ticket/23406 migration file
> discovery was changed to use `pkgutil. iter_modules ()`, instead of
> direct filesystem access. `pkgutil. iter_modules()` uses the package's
> `__path__` list, which exists on implicit namespace packages.
>
> As a result, the `__file__` check is no longer needed, and in fact
> prevents `migrate` from working on namespace packages (implicit or
> otherwise).
>
> Related work: https://code.djangoproject.com/ticket/29091
New description:
Background: In python 3 a package with no `__init__.py` is implicitly a
namespace package, so it has no `__file__` attribute.
The migrate command currently checks for existence of a `__file__`
attribute on the `migrations` package. This check was introduced in
#21015, because the `__file__` attribute was used in migration file
discovery.
However, in #23406 migration file discovery was changed to use
`pkgutil.iter_modules ()`, instead of direct filesystem access. `pkgutil.
iter_modules()` uses the package's `__path__` list, which exists on
implicit namespace packages.
As a result, the `__file__` check is no longer needed, and in fact
prevents `migrate` from working on namespace packages (implicit or
otherwise).
Related work: #29091
--
Comment (by Tim Graham):
What's the use case? When `makemigrations` generates the first migration,
it adds an `__init__.py` file.
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Benjy Weinberger):
The "or otherwise" refers to namespace packages being either implicit
(lacking an __init__.py) or explicit (an __init__.py containing an old-
style namespace package incantation).
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Benjy Weinberger):
Re the use case:
It seems unnecessary to insist on an empty `__init__.py` just to preserve
a (stale) check for `__file__`, when they are otherwise not required in
Python 3.
And https://code.djangoproject.com/ticket/29091 implies that this should
be allowed.
Context is: We've just finished migrating our codebase to all-python 3,
and nuked all our `__init__py` files. We are now upgrading to Django 2.1,
and noticing the various places where they're still de-facto required by
Django. This particular case seemed unnecessary and easy to fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Václav Řehák):
We had a similar issue with test discovery. One of our developers read an
article that `__init__.py` files are not required on Python3 and started
removing them. Everything seemingly worked but some tests were not
discovered and did not run in CI (which made it difficult to spot).
I think Django should not required them if technically possible or at
least it should be made clear in the documentation (sorry if I overlooked
it).
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: assigned
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Benjy Weinberger):
Friendly nudge?
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* version: 2.1 => master
Comment:
[https://github.com/django/django/pull/11141 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3cd3bebe8921e14b911b36b2a1cbceef8fb6294e" 3cd3bebe]:
{{{
#!CommitTicketReference repository=""
revision="3cd3bebe8921e14b911b36b2a1cbceef8fb6294e"
Fixed #30300 -- Allowed migrations to be loaded from directories without
__init__.py file.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I raised this issue [https://groups.google.com/g/django-
developers/c/GVHMH2ciAnk/m/vbIPbZuSBQAJ on the mailing list] as there was
a thread that concluded that Django shouldn't promote use of implicit
namespace packages.
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* version: master => 3.1
* resolution: fixed =>
* severity: Normal => Release blocker
Comment:
Thus far there haven't been any mailing list replies in favor this change.
I created a [https://github.com/django/django/pull/13218 PR] to revert it
and modify the test accordingly.
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Component: Database layer | Version: 3.1
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"ff55adbd0da6618abaf265d16196bf54f81aa77a" ff55adbd]:
{{{
#!CommitTicketReference repository=""
revision="ff55adbd0da6618abaf265d16196bf54f81aa77a"
Reverted "Fixed #30300 -- Allowed migrations to be loaded from directories
without __init__.py file."
This reverts commit 3cd3bebe8921e14b911b36b2a1cbceef8fb6294e.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"d004bcee0c6c5558e524e2f5c75e3d70a119fd3d" d004bce]:
{{{
#!CommitTicketReference repository=""
revision="d004bcee0c6c5558e524e2f5c75e3d70a119fd3d"
[3.1.x] Reverted "Fixed #30300 -- Allowed migrations to be loaded from
directories without __init__.py file."
This reverts commit 3cd3bebe8921e14b911b36b2a1cbceef8fb6294e.
Backport of ff55adbd0da6618abaf265d16196bf54f81aa77a from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Benjy Weinberger | Owner: Benjy
| Weinberger
Type: New feature | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: fixed => wontfix
* stage: Accepted => Unreviewed
--
Ticket URL: <https://code.djangoproject.com/ticket/30300#comment:16>