[Django] #24559: MigrationLoader.load_disk ImportError handling differs under py3/pypy vs py2

14 views
Skip to first unread message

Django

unread,
Apr 1, 2015, 3:18:31 AM4/1/15
to django-...@googlegroups.com
#24559: MigrationLoader.load_disk ImportError handling differs under py3/pypy vs
py2
----------------------------+--------------------
Reporter: kezabelle | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
Currently, within `load_disk`, the following happens:

{{{
try:
module = import_module(module_name)
except ImportError as e:
# I hate doing this, but I don't want to squash other
import errors.
# Might be better to try a directory check directly.
if "No module named" in str(e) and MIGRATIONS_MODULE_NAME
in str(e):
self.unmigrated_apps.add(app_config.label)
continue
raise
}}}

However, the
[https://github.com/django/django/blob/2832a9b028c267997b2fd3dd0989670d57cdd08f/django/db/migrations/loader.py#L73
if statement] behaves differently under pypy & python3, because the
exception string is different.

Under python2, the following yields
{{{
>>> from importlib import import_module
>>> importlib('test_auth.migrations')
ImportError: No module named test_auth.migrations
}}}

which would hit both parts of the conditional, and end up in
`self.unmigrated_apps`

However, under python3.3.3 the exception is:
{{{
ImportError: No module named 'test_auth'
}}}
and under pypy 2.2.1 (for python 2.7.3) the exception is:
{{{
ImportError: No module named test_auth
}}}
(same as py3k, sans quotes)

Neither of these would end up `self.unmigrated_apps` because
`MIGRATIONS_MODULE_NAME` doesn't appear in the string representation,
though I suspect `import_module('testauthmigrations')` '''would''' work.

I originally noticed this because of a [https://travis-ci.org/kezabelle
/django-typesafetynet/builds/55545694 travis build] for a toy project, so
it may not be a problem in practice, but it is at least inconsistent.

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

Django

unread,
Apr 1, 2015, 7:44:09 AM4/1/15
to django-...@googlegroups.com
#24559: MigrationLoader.load_disk ImportError handling differs under py3/pypy vs
py2
----------------------------+------------------------------------

Reporter: kezabelle | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
----------------------------+------------------------------------
Changes (by timgraham):

* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted


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

Django

unread,
Apr 19, 2020, 11:59:42 AM4/19/20
to django-...@googlegroups.com
#24559: MigrationLoader.load_disk ImportError handling differs under py3/pypy vs
py2
------------------------------+------------------------------------
Reporter: Keryn Knight | Owner: nobody

Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by Jon Dufresne):

* has_patch: 0 => 1


Comment:

https://github.com/django/django/pull/12756

Python 2 is no longer suported, but this bug report shows why inspecting
an exception's message isn't a stable API. The above PR changes the code
to use the exception type and its "name" property.

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

Django

unread,
Apr 20, 2020, 1:26:23 AM4/20/20
to django-...@googlegroups.com
#24559: MigrationLoader.load_disk ImportError handling differs under py3/pypy vs
py2
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Jon
| Dufresne
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* owner: nobody => Jon Dufresne
* status: new => assigned
* stage: Accepted => Ready for checkin


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

Django

unread,
Apr 20, 2020, 1:48:28 AM4/20/20
to django-...@googlegroups.com
#24559: MigrationLoader.load_disk ImportError handling differs under py3/pypy vs
py2
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Jon
| Dufresne
Type: Bug | Status: closed
Component: Migrations | Version: master
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: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

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


Comment:

In [changeset:"661e39c8d5bb05d8ebe8492912bf9f63453b279f" 661e39c8]:
{{{
#!CommitTicketReference repository=""
revision="661e39c8d5bb05d8ebe8492912bf9f63453b279f"
Fixed #24559 -- Made MigrationLoader.load_disk() catch more specific
ModuleNotFoundError.

Avoids inspecting the exception message, which is not considered a
stable API and can change across Python versions.

ModuleNotFoundError was introduced in Python 3.6. It is a subclass of
ImportError that is raised when the imported module does not exist. It
is not raised for other errors that can occur during an import. This
exception instance has the property "name" which holds the name of
module that failed to import.
}}}

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

Reply all
Reply to author
Forward
0 new messages