Opened 4 years ago

Closed 4 years ago

#32350 closed Bug (fixed)

Showmigrations applied timestamp can cause runtime error

Reported by: Daniel Ebrahimian Owned by: Daniel Ebrahimian
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: management command showmigrations applied timestamp
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Daniel Ebrahimian)

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)

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

Change History (10)

comment:1 by Daniel Ebrahimian, 4 years ago

Description: modified (diff)

comment:2 by Daniel Ebrahimian, 4 years ago

Owner: changed from nobody to Daniel Ebrahimian
Status: newassigned

comment:3 by Daniel Ebrahimian, 4 years ago

Version: 3.1master

comment:4 by Simon Charette, 4 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

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

comment:5 by Daniel Ebrahimian, 4 years ago

Thanks for the feedback. I have added unit tests for this change. Please review

comment:6 by Daniel Ebrahimian, 4 years ago

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.

Version 0, edited 4 years ago by Daniel Ebrahimian (next)

comment:7 by Daniel Ebrahimian, 4 years ago

I have pushed changes, CI is now passing. @Simon Charette

comment:8 by Simon Charette, 4 years ago

Needs tests: unset
Patch needs improvement: unset

LGTM pending some cosmetic changes. The stdout pollution when running tests is handled by #32395.

comment:9 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 3f8979e3:

Fixed #32350 -- Fixed showmigrations crash for applied squashed migrations.

Thanks Simon Charette for reviews.

Note: See TracTickets for help on using tickets.
Back to Top