Opened 2 years ago

Closed 2 years ago

#28523 closed Cleanup/optimization (wontfix)

Improve the error message when squashing migration with RunPython that require manual changes

Reported by: Václav Řehák Owned by: nobody
Component: Migrations Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

#22983 fixed the need to manually modify squashed migration with RunPython by providing explanation in the squasmigrations output and as a comment on the top of the migration file. While this should be enough for careful reader I missed it and spent some time investigating why migrations are giving me a syntax error:

  File "/home/vaclav/project/src/core/migrations/0002_user_authentication_squashed_0056_my_migration.py", line 84
    code=core.migrations.0005_add_social_providers.create_providers,
                            ^
SyntaxError: invalid syntax

The problem is that if you open the squashed migration file on the line with error, you start thinking why Django is creating file with invalid Python syntax and miss the comment on the top of the file.

I'm proposing a simple change to migration which require manual change to make it more obvious. I'm not sure what the right form would be but e.g. placing this right after the comment makes the developers experience much better:

from django.core.exceptions import ImproperlyConfigured                                                                 
raise ImproperlyConfigured("Manual change to migration required")


Now if I run "manage.py migrate" I get a clear error message what is wrong:

  File "/home/vaclav/project/src/core/migrations/0002_user_authentication_squashed_0056_my_migration.py", line 33, in <module>
    raise ImproperlyConfigured("Manual change to migration required")
django.core.exceptions.ImproperlyConfigured: Manual change to migration required

Change History (3)

comment:1 Changed 2 years ago by Václav Řehák

Summary: Better error message for #22983 (squashed migration with RunPython) https://code.djangoproject.com/ticket/22983 fixed the need to manually modify squashed migration with RunPython by providing explanation in the squasmigrations output and as a comment on the top of the migration file. While this should be enough for careful reader I missed it and spent some time investigating why migration are giving me a syntax error: File "/home/vaclav/project/src/core/migrations/0002_user_authentication_squashed_0056_my_migration.py", line 84 code=core.migrations.0005_add_social_providers.create_providers, ^ SyntaxError: invalid syntax The problem is that if you open the squashed migration file on the line with error, you start thinking why Django is creating file with invalid Python syntax and miss the comment on the top of the file. I'm proposing a simple change to migration which require manual change to make it more obvious. I'm not sure what the right form would be but e.g. placing this right after the comment makes the developers experience much better: from django.core.exceptions import ImproperlyConfigured raise ImproperlyConfigured("Manual change to migration required")Better error message for #22983 (squashed migration with RunPython)

comment:2 Changed 2 years ago by Tim Graham

Easy pickings: unset
Summary: Better error message for #22983 (squashed migration with RunPython)Improve the error message when squashing migration with RunPython that require manual changes
Type: New featureCleanup/optimization

I'm not sure how much your proposal would help in your case. If the migration contains a syntax error, it won't be able to run the code to raise ImproperlyConfigured. In light of that, I'm not sure if the proposed change is worth the additional complexity.

comment:3 Changed 2 years ago by Václav Řehák

Resolution: wontfix
Status: newclosed

OK, agreed that more complex change would be required to fix the syntax error and it's not worth the effort.

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