Opened 7 years ago

Closed 7 years ago

#26292 closed Cleanup/optimization (fixed)

Refactor MigrationLoader.build_graph() to use more of MigrationGraph's features

Reported by: Markus Holtermann Owned by: Jarek Glowacki
Component: Migrations Version: dev
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

The build_graph method on the MigrationWriter is rather complex and hard to understand. While talking to @jarekwg about #25945 we came to the conclusion that it would be worth to see if moving parts of that method into the MigrationGraph would be suitable.

General idea:

  1. Add all normal migrations
  2. Add all their dependencies (create dummy nodes for non-existing squashed migrations)
  3. Replace the nodes in the graph when they have been replaces with suitable squashed migrations and update the dependencies accordingly
  4. Ensure there are no dummy nodes left in the graph and the graph is consistent.

Change History (6)

comment:1 Changed 7 years ago by Jarek Glowacki

While working on this, I started toying around with the idea of building the migration graph in a more complex way where it's aware of applied/non-applied and replacing/non-replacing migrations. Something along the lines of a directional graph with coloured nodes (denoting applied/non-applied) and coloured edges (denoting dependency/replaces).

This would still spawn dummy nodes as it builds the graph, but once finished, an extra function could then be run over the graph - something like simplify or resolve - where the graph would resolve all the replacing migrations internally along with validating that no dummy migrations remain, simplifying itself back down to the form it has currently.

It's not the path I'm taking currently, as I'm trying to avoid giving the graph too much knowledge of stuff that's only needed for its initialisation. Seems to belong more to the loader. But just thought I'd leave this idea here as food for thought.

Last edited 7 years ago by Jarek Glowacki (previous) (diff)

comment:2 Changed 7 years ago by Jarek Glowacki

Has patch: set

comment:3 Changed 7 years ago by Tim Graham

Patch needs improvement: set

comment:4 Changed 7 years ago by Markus Holtermann

Patch needs improvement: unset

I'll give this PR a small benchmark test later this week, but would generally love to see this in 1.10. There are a couple of optimization options Jarek and I have in mind but they should be fine to be submitted when alpha was cut, IMO.

comment:5 Changed 7 years ago by Markus Holtermann

Triage Stage: AcceptedReady for checkin

Optimization is tracked in #26407.

comment:6 Changed 7 years ago by Markus Holtermann <info@…>

Resolution: fixed
Status: newclosed

In 509379a1:

Fixed #25945, #26292 -- Refactored MigrationLoader.build_graph()

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