Opened 7 years ago
Closed 3 years ago
#29063 closed Bug (fixed)
Naming an incompletely applied squashed migration as a migration target fails with bare NodeNotFoundError
Reported by: | Julian | Owned by: | Jacob Walls |
---|---|---|---|
Component: | Migrations | Version: | 1.9 |
Severity: | Normal | Keywords: | |
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
In Line 205-208 in django/db/migrations/loader.py replacement migrations (created with squash) are checked if they can be applied. If any of the to be replaced migrations isn't already applied the replacement migration is not added to the nodes list.
This leads to the fact that if some of the migrations are removed or not completely applied before the squash is added and there is a dependency on the replacement migration, the user gets a 'NodeNotFoundError' where the replacement migration that is not being applied because of line 206 is the missing one.
This is very confusing to the user, raising a warning in line 208 would inform the user that the squashed migration can not be applied because not all the 'child' migrations are applied.
Had to debug into that to figure that out.
Change History (9)
comment:1 by , 7 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 3 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:5 by , 3 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
On the PR Mariusz demonstrated several examples having to do with this ticket's description where the error messages are sufficiently informative. However, I think my test case is related to the original poster's scenario (although it is hard to be certain), and shows Django raising an improvable NodeNotFoundError
.
Glad to give it another look, though, if you think I'm misreading the original poster's use case. (To that end, Julian, I would be grateful if you would be able to take a look at my suggested patch.)
comment:6 by , 3 years ago
Type: | Cleanup/optimization → Bug |
---|
After Mariusz helpfully enumerated scenarios where the current messaging is sufficient, we found really only a single case with an improvable NodeNotFoundError
, and I agree it would be better to just fix the failure point than invest energy in a patch changing the exception message:
comment:7 by , 3 years ago
Summary: | Replacement Migrations not being executed because of unapplied migrations should raise a warning. → Naming an incompletely applied squashed migration as a migration target fails with bare NodeNotFoundError |
---|
comment:8 by , 3 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Somewhat similar to #23556, I think we can raise a more informative
NodeNotFoundError
. Should have a patch soon, just need to toy with whether backward migrations are also a problem.