#30300 closed New feature (wontfix)
Allow migrations directories without __init__.py files
Reported by: | Benjy Weinberger | Owned by: | Benjy Weinberger |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.1 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
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
Change History (16)
comment:1 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 6 years ago
comment:3 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → New feature |
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?
comment:4 by , 6 years ago
Summary: | migrate silently ignores apps whose migrations directory doesn't have an __init__.py → Allow migrations directories without __init__.py files. |
---|
comment:5 by , 6 years ago
Description: | modified (diff) |
---|---|
Summary: | Allow migrations directories without __init__.py files. → Allow migrations directories without __init__.py files |
What's the use case? When makemigrations
generates the first migration, it adds an __init__.py
file. There was some concern about encouraging namespace packages in ticket:23919#comment:102.
comment:6 by , 6 years ago
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).
comment:7 by , 6 years ago
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 #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.
comment:8 by , 6 years ago
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).
comment:12 by , 4 years ago
I raised this issue on the mailing list as there was a thread that concluded that Django shouldn't promote use of implicit namespace packages.
comment:13 by , 4 years ago
Resolution: | fixed |
---|---|
Severity: | Normal → Release blocker |
Status: | closed → new |
Version: | master → 3.1 |
Thus far there haven't been any mailing list replies in favor this change. I created a PR to revert it and modify the test accordingly.
comment:16 by , 4 years ago
Resolution: | fixed → wontfix |
---|---|
Triage Stage: | Accepted → Unreviewed |
Proposed fix in https://github.com/django/django/pull/11141
(branch https://github.com/benjyw/django/tree/ticket_30300)/