[Django] #26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's features

8 views
Skip to first unread message

Django

unread,
Feb 28, 2016, 9:25:47 PM2/28/16
to django-...@googlegroups.com
#26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's
features
------------------------------------------------+-------------------------
Reporter: MarkusH | Owner: jarekwg
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+-------------------------
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
1. Add all their dependencies (create dummy nodes for non-existing
squashed migrations)
1. Replace the nodes in the graph when they have been replaces with
suitable squashed migrations and update the dependencies accordingly
1. Ensure there are no dummy nodes left in the graph and the graph is
consistent.

--
Ticket URL: <https://code.djangoproject.com/ticket/26292>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 6, 2016, 6:43:49 PM3/6/16
to django-...@googlegroups.com
#26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's
features
--------------------------------------+------------------------------------

Reporter: MarkusH | Owner: jarekwg
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by jarekwg):

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

--
Ticket URL: <https://code.djangoproject.com/ticket/26292#comment:1>

Django

unread,
Mar 8, 2016, 10:28:04 PM3/8/16
to django-...@googlegroups.com
#26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's
features
--------------------------------------+------------------------------------

Reporter: MarkusH | Owner: jarekwg
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by jarekwg):

* has_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/26292#comment:2>

Django

unread,
Mar 9, 2016, 7:52:27 PM3/9/16
to django-...@googlegroups.com
#26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's
features
--------------------------------------+------------------------------------

Reporter: MarkusH | Owner: jarekwg
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by timgraham):

* needs_better_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/26292#comment:3>

Django

unread,
May 3, 2016, 6:38:02 AM5/3/16
to django-...@googlegroups.com
#26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's
features
--------------------------------------+------------------------------------

Reporter: MarkusH | Owner: jarekwg
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by MarkusH):

* needs_better_patch: 1 => 0


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/26292#comment:4>

Django

unread,
May 8, 2016, 7:39:07 AM5/8/16
to django-...@googlegroups.com
#26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's
features
-------------------------------------+-------------------------------------
Reporter: MarkusH | Owner: jarekwg
Type: | Status: new
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by MarkusH):

* stage: Accepted => Ready for checkin


Comment:

Optimization is tracked in #26407.

--
Ticket URL: <https://code.djangoproject.com/ticket/26292#comment:5>

Django

unread,
May 8, 2016, 7:54:54 AM5/8/16
to django-...@googlegroups.com
#26292: Refactor MigrationLoader.build_graph() to use more of MigrationGraph's
features
-------------------------------------+-------------------------------------
Reporter: MarkusH | Owner: jarekwg
Type: | Status: closed
Cleanup/optimization |
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Markus Holtermann <info@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"509379a161d179f9e973fbaf6393e879531756f6" 509379a1]:
{{{
#!CommitTicketReference repository=""
revision="509379a161d179f9e973fbaf6393e879531756f6"
Fixed #25945, #26292 -- Refactored MigrationLoader.build_graph()
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26292#comment:6>

Reply all
Reply to author
Forward
0 new messages