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 )
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)
Change History (10)
comment:1 by , 4 years ago
Description: | modified (diff) |
---|
comment:2 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 4 years ago
Version: | 3.1 → master |
---|
comment:4 by , 4 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
comment:5 by , 4 years ago
Thanks for the feedback. I have added unit tests for this change. Please review
comment:6 by , 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.
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.
comment:8 by , 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 , 4 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
It looks like the issue is related to the usage of
replaces
which is relied upon thesquashmigrations
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 replacedjango-social-auth
migrationse.g. https://github.com/python-social-auth/social-app-django/blob/7a105df36ca579389b93911f748ee297ce141bb3/social_django/migrations/0001_initial.py#L29-L32