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
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