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 Tim Graham, 7 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Jacob Walls, 3 years ago

Owner: changed from nobody to Jacob Walls
Status: newassigned

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.

comment:3 by Jacob Walls, 3 years ago

Has patch: set

comment:4 by Mariusz Felisiak, 3 years ago

Needs tests: set
Patch needs improvement: set

comment:5 by Jacob Walls, 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 Jacob Walls, 3 years ago

Type: Cleanup/optimizationBug

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:

New PR

comment:7 by Jacob Walls, 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 Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 910ecd1:

Fixed #29063 -- Fixed migrate crash when specifying a name of partially applied squashed migrations.

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