[Django] #32350: Showmigrations applied timestamp can cause runtime error
Django
-------------------------------------+-------------------------------------
Reporter: Daniel | Owner: nobody
Ebrahimian |
Type: Bug | Status: new
Component: Core | Version: 3.1
(Management commands) | Keywords: management command
Severity: Normal | showmigrations applied timestamp
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The functionality of `showmigrations --verbosity 2` will show the
`applied` timestamps for migrations.
In certain scenarios, a timestamp isn't available and will cause a run
time error of
`AttributeError: 'Migration' object has no attribute 'applied'`.
An example of this is having `social_django` migrations in a project.
Proposed solution: Safely check for the `applied` timestamp to exist
before logging it.
Current workaround: Don't use `--verbosity 2`
Logs
This shows the type for the `applied_migration`
{{{
<class
'django.db.migrations.recorder.MigrationRecorder.Migration.<locals>.Migration'>
[X] 0045_auto_20201125_0810 (applied at 2021-01-13 23:39:40)
<class
'django.db.migrations.recorder.MigrationRecorder.Migration.<locals>.Migration'>
[X] 0046_auto_20201218_0440 (applied at 2021-01-13 23:39:42)
sessions
<class
'django.db.migrations.recorder.MigrationRecorder.Migration.<locals>.Migration'>
[X] 0001_initial (applied at 2021-01-13 23:40:46)
social_django
<class 'social_django.migrations.0001_initial.Migration'>
Traceback (most recent call last):
File "real_manage.py", line 23, in <module>
execute_from_command_line(sys.argv)
File "/usr/local/lib/python3.8/site-
packages/django/core/management/__init__.py", line 401, in
execute_from_command_line
utility.execute()
File "/usr/local/lib/python3.8/site-
packages/django/core/management/__init__.py", line 395, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/usr/local/lib/python3.8/site-
packages/django/core/management/base.py", line 328, in run_from_argv
self.execute(*args, **cmd_options)
File "/usr/local/lib/python3.8/site-
packages/django/core/management/base.py", line 369, in execute
output = self.handle(*args, **options)
File "/usr/local/lib/python3.8/site-
packages/django/core/management/commands/showmigrations.py", line 52, in
handle
return self.show_list(connection, options['app_label'])
File "/usr/local/lib/python3.8/site-
packages/django/core/management/commands/showmigrations.py", line 97, in
show_list
output += ' (applied at %s)' %
applied_migration.applied.strftime('%Y-%m-%d %H:%M:%S')
AttributeError: 'Migration' object has no attribute 'applied'
}}}
This is after the proposed pull request
{{{
[X] 0045_auto_20201125_0810 (applied at 2021-01-13 23:39:40)
<class
'django.db.migrations.recorder.MigrationRecorder.Migration.<locals>.Migration'>
[X] 0046_auto_20201218_0440 (applied at 2021-01-13 23:39:42)
sessions
<class
'django.db.migrations.recorder.MigrationRecorder.Migration.<locals>.Migration'>
[X] 0001_initial (applied at 2021-01-13 23:40:46)
social_django
<class 'social_django.migrations.0001_initial.Migration'>
[X] 0001_initial (2 squashed migrations)
<class 'social_django.migrations.0002_add_related_name.Migration'>
[X] 0002_add_related_name (2 squashed migrations)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32350>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Core (Management | Version: 3.1
commands) |
Severity: Normal | Resolution:
Keywords: management command | Triage Stage:
showmigrations applied timestamp | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
Logs
[https://github.com/django/django/pull/13890]
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:1>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Type: Bug | Status: assigned
Component: Core (Management | Version: 3.1
commands) |
Severity: Normal | Resolution:
Keywords: management command | Triage Stage:
showmigrations applied timestamp | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Daniel Ebrahimian
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:2>
Django
-------------------------------------+-------------------------------------
| danielebra
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: management command | Triage Stage:
showmigrations applied timestamp | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 3.1 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:3>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
showmigrations applied timestamp |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
It looks like the issue is related to the usage of `replaces` which is
relied upon the `squashmigrations` feature.
Tentatively accepting if you're able to write a test that triggers a crash
when relying on a supported usage of `replaces` (e.g. a migration squash).
It looks like `social-app-django` ships with a complex graph of migrations
to ''replace'' `django-social-auth` migrations
e.g. https://github.com/python-social-auth/social-app-
django/blob/7a105df36ca579389b93911f748ee297ce141bb3/social_django/migrations/0001_initial.py#L29-L32
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:4>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: management command | Triage Stage: Accepted
showmigrations applied timestamp |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Comment (by Daniel Ebrahimian):
Thanks for the feedback. I have added unit tests for this change. Please
review
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:5>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: management command | Triage Stage: Accepted
showmigrations applied timestamp |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Comment (by Daniel Ebrahimian):
When running the unit tests locally, the output contains terminal colour
escape sequences. I have included them when writing the unit tests.
However, when picked up by CI, the output differs.
https://djangoci.com/job/pr-
mariadb/database=mysql,label=mariadb,python=python3.9/9872/testReport/junit/migrations.test_commands/MigrateTests/test_showmigrations_list_squashed/
I have executed tests locally with
`python runtests.py
migrations.test_commands.MigrateTests.test_showmigrations_list_squashed`
```sh
Testing against Django installed in
'/Users/daniel/Documents/GenesisCare/projects/django/django'
Creating test database for alias 'default'...
Creating test database for alias 'other'...
System check identified no issues (0 silenced).
Running pre-migrate handlers for application migrations
Running post-migrate handlers for application migrations
.
----------------------------------------------------------------------
Ran 1 test in 0.017s
OK
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
```
Please advise whether these codes should be removed and how to then
reproduce CI testing practices locally.
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:6>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: management command | Triage Stage: Accepted
showmigrations applied timestamp |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Comment (by Daniel Ebrahimian):
I have pushed changes, CI is now passing. @Simon Charette
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:7>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
Keywords: management command | Triage Stage: Accepted
showmigrations applied timestamp |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
Comment:
LGTM pending some cosmetic changes. The stdout pollution when running
tests is handled by #32395.
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:8>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Type: Bug | Status: assigned
Component: Core (Management | Version: master
commands) |
Severity: Normal | Resolution:
showmigrations applied timestamp | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:9>
Django
-------------------------------------+-------------------------------------
| Ebrahimian
Component: Core (Management | Version: master
commands) |
Keywords: management command | Triage Stage: Ready for
showmigrations applied timestamp | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3f8979e37b6c498101d09a583ccc521d7f2879e5" 3f8979e3]:
{{{
#!CommitTicketReference repository=""
revision="3f8979e37b6c498101d09a583ccc521d7f2879e5"
Fixed #32350 -- Fixed showmigrations crash for applied squashed
migrations.
Thanks Simon Charette for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32350#comment:10>