Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2020-04-08 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  closed
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by felixxm):

 * status:  assigned => closed
 * needs_better_patch:  1 => 0
 * resolution:   => fixed


Comment:

 Fixed in 1d16c5d562a67625f7309cc7765b8c57d49bf182.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.ce1a44046658cf0c3aa9ea00d4da2be0%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2020-04-08 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by GitHub ):

 In [changeset:"db6933a032c850153a688b6c977691b37ca02745" db6933a]:
 {{{
 #!CommitTicketReference repository=""
 revision="db6933a032c850153a688b6c977691b37ca02745"
 Refs #30966 -- Added test for reloading related model state on non-
 relational changes.

 Thanks Markus Holtermann for initial test.

 Fixed in 1d16c5d562a67625f7309cc7765b8c57d49bf182.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.d0f11f6f4829308e64317dc688a804c1%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2020-04-07 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"1d16c5d562a67625f7309cc7765b8c57d49bf182" 1d16c5d]:
 {{{
 #!CommitTicketReference repository=""
 revision="1d16c5d562a67625f7309cc7765b8c57d49bf182"
 Refs #27666 -- Ensured relationship consistency on delayed reloads.

 Delayed reloads of state models broke identity based relationships
 between direct and non-direct ancestors.

 Basing models.Options related objects map of model labels instead of
 their identity ensured relationship consistency is maintained.

 Refs #30966.

 Co-authored-by: Hasan Ramezani 
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.7dd72610ae01f8f75b9c47a252846d7d%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2020-04-04 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 Submitted [https://github.com/django/django/pull/12665 a PR] to
 demonstrate it properly addresses #29000 and passes the suite.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.af691374b33419f7d5545b1cf8a7a1c0%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2020-04-04 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 After doing a bit of investigation work for #31064 and revisiting #29000 I
 think the solution proposed by Hasan makes the most sense for now and
 here's why.

 Let's take the setup defined to test #29000 as an example

 {{{#!python
 class Pony(models.Model):
 name = models.CharField(max_length=20)

 class Rider(models.Model):
pony = models.ForeignKey('test_rename_multiple.Pony', models.CASCADE)

 class PonyRider(models.Model):
riders = models.ManyToManyField('Rider')
 }}}

 If a `RenameField('Pony', 'name', 'fancy_field)` operation is applied and
 `Pony` is rendered from the new state only the `Rider.pony` pointer is
 actually stale and needs to be repointed.

 However we decided not to follow the repointing route because of all the
 trouble it incurs. Instead we re-render only the direct relationships if
 `delay=True` (only `Rider` in this case) or the full relationship tree if
 `delay=False` (`Rider` and `PonyRider` in this case). A full relationship
 tree re-rendering is slow hence why we try to use `delay=True` as much as
 possible but when we do so references to direct relationships of the
 reloaded models are left pointing at pre-re-rendering `Model` classes
 which breaks `Options._populate_directed_relation_graph` mapping because
 it currently relies on `Options` identity as key.

 To reuse our example setup above, if `PonyRider -> Rider -> Pony` and we
 `reload_model('app', 'pony', delay=True)` then `Pony` and `Rider` will be
 reloaded as `Rider' -> Pony'` but `PonyRider` will still point at `Pony`
 and not `Pony'` which makes any following operation relying on
 `Pony'._meta.related_objects` break because `Pony'._meta is not
 Pony._meta`.

 If we look at how #29000 was addressed
 
[https://github.com/django/django/commit/fcc4e251dbc917118f73d7187ee2f4cbf3883f36
 #diff-efb7d00c2383393046c9c5d842f45499R290 we can see that it works around
 this issue by forcing a full relationship tree re-rendering as soon as any
 model refers to a model on which a field is renamed]. This is really
 wasteful but it gets the test passing in this particular case which is
 more a bandaid than anything else. Fundamentally the `delay=True` model if
 broken if it doesn't allow non-re-rendered models to maintain some
 relationship with the re-rendered graph.

 For these reasons and because model identity consistency is not necessary
 beyond direct relationship by the current schema editor I think that
 adjusting `Options._populate_directed_relation_graph` to use
 `Options.label` as relationship key is our best way forward here as it
 doesn't hurt non-`__fake__` model relationship resolution and it allows
 `StateApps` to keep relying on the `delay=True` for model rendering until
 schema alterations don't necessitate access to `ProjectState.apps`
 anymore.

 I'll submit a PR that reverts unnecessary `RenameField.state_forwards`
 changes introduced by fcc4e251dbc917118f73d7187ee2f4cbf3883f36 and use
 Hasan's `_populate_directed_relation_graph` adjustments to demonstrate
 that the tests are passing. From that point, if everyone agrees that this
 is the right way forward, I think we should keep this ticket opened as a
 marker to add regression tests showing this is was properly addressed.

 Thoughts on that approach Markus and Hasan?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.b34def39e4405d07706615cca19a12e9%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Hasan Ramezani):

 Thanks Markus for the patch.

 If there is something that can be done by me, please let me know.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.3983e02902735c0d04b5855da0309864%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 Thanks Markus.

 The patch will likely need a bit of polishing but a solution will likely
 be out best shot until we only deal with model states.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.082ec7a4b21a9cc1e6e6039edbfea69d%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Markus Holtermann):

 I took a shot at implementing something along the lines of what I
 suggested in 2. in [comment:13]:
 https://github.com/MarkusH/django/pull/new/ticket30966 . At this point it
 does not handle unapplying of migrations.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.bd03348d1283caa2355b71ce014affab%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Markus Holtermann):

 Replying to [comment:15 Peter Thomassen]:
 > Replying to [comment:13 Markus Holtermann]:
 > > 4. Leave as-is; the first migration can be optimized anyway
 >
 > The first migration can trivially be optimized in this minimum example.
 In our real example, we encountered the problem where migration 0003
 interferes with migration 0011, with existing deployments somewhere in the
 middle.
 >
 > In that case, 0003 and 0011 can be optimized/squashed more or less
 easily for new deployments, but migration 0011 would have to be maintained
 individually for those deployments which already have all migrations up to
 that one applied. The result would be code duplication for "fresh" and
 "existing" deployments, and -- if this problem reoccurs -- maybe even more
 "squashed migration variations".
 >
 > So far, squashing was optional, and it would become a mandatory
 mechanism, while producing code duplication in cases as described above.
 In my opinion, that would be the least favorable option.

 Yeah, that'd be true. As said, none of this is really ideal.

 What we're doing here are unfortunately patches upon patches upon patches.
 I'd really like to move onto the "right" way.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.26abf56100af9d150c411a2220192668%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Markus Holtermann):

 This is a minimal failing test
 {{{#!diff
 diff --git a/tests/migrations/test_state.py
 b/tests/migrations/test_state.py
 index e9bdac93c5..9140609bb6 100644
 --- a/tests/migrations/test_state.py
 +++ b/tests/migrations/test_state.py
 @@ -928,6 +928,64 @@ class StateTests(SimpleTestCase):
  choices_field = Author._meta.get_field('choice')
  self.assertEqual(list(choices_field.choices), choices)

 +def test_reload_related_models_on_non_relational_fields(self):
 +"""
 +#30966 - Even when fields on a model change that are not involved
 in
 +a relation, the model gets reloaded. Ensure that other models
 pointing
 +to or from it are reloaded accordingly.
 +
 +User  <--  Domain  <-- RRset
 +  ^
 +  +--  Token
 +"""
 +project_state = ProjectState()
 +# Render project state to simulate initial migration state
 +project_state.apps
 +project_state.add_model(ModelState(
 +"migrations",
 +"User",
 +[
 +("id", models.AutoField(primary_key=True)),
 +("email", models.EmailField(max_length=191,
 unique=True)),
 +]
 +))
 +project_state.add_model(ModelState(
 +"migrations",
 +"Domain",
 +[
 +("id", models.AutoField(primary_key=True)),
 +("owner", models.ForeignKey("migrations.User",
 models.SET_NULL)),
 +]
 +))
 +project_state.add_model(ModelState(
 +"migrations",
 +"RRset",
 +[
 +("id", models.AutoField(primary_key=True)),
 +("subname", models.CharField(blank=True,
 max_length=178)),
 +("domain", models.ForeignKey("migrations.Domain",
 models.SET_NULL)),
 +]
 +))
 +project_state.add_model(ModelState(
 +"migrations",
 +"Token",
 +[
 +("id", models.AutoField(primary_key=True)),
 +("user", models.ForeignKey("migrations.User",
 models.SET_NULL)),
 +]
 +))
 +AlterField(
 +model_name="RRset",
 +name="subname",
 +field=models.CharField(blank=False, max_length=178),
 +).state_forwards("migrations", project_state)
 +
 +all_models = project_state.apps.all_models["migrations"]
 +assert (
 +all_models['token']._meta.get_field('user').related_model
 +is all_models['user']
 +), "Models are not identical"
 +

  class ModelStateTests(SimpleTestCase):
  def test_custom_model_base(self):
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.cca411c8c889ff69665aeaff0c292eb1%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Peter Thomassen):

 Replying to [comment:13 Markus Holtermann]:
 > 4. Leave as-is; the first migration can be optimized anyway

 The first migration can trivially be optimized in this minimum example. In
 our real example, we encountered the problem where migration 0003
 interferes with migration 0011, with existing deployments somewhere in the
 middle.

 In that case, 0003 and 0011 can be optimized/squashed more or less easily
 for new deployments, but migration 0011 would have to be maintained
 individually for those deployments which already have all migrations up to
 that one applied. The result would be code duplication for "fresh" and
 "existing" deployments, and -- if this problem reoccurs -- maybe even more
 "squashed migration variations".

 So far, squashing was optional, and it would become a mandatory mechanism,
 while producing code duplication in cases as described above. In my
 opinion, that would be the least favorable option.

 Thanks a LOT for your time, and for thinking though this whole thing!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.e57cd9ff2a51d1a8996dccafae5ab6fd%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Markus Holtermann):

 Replying to [comment:12 Hasan Ramezani]:

 > this is an alter migration and the `AlterField.state_forwards` reloads
 3 models of 4 (User, Domain, RRest) and don't reload the Token model.
 > If I reload the Token model this test will be passed.
 >
 >
 > Should I change the function to find proper models to reload?
 > Is it the right solution?

 This is essentially approach 1 from my list in [comment:13]

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.b7553469559fddbade447ccc471c0530%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-15 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Markus Holtermann):

 As already suspected by Simon, this is indeed related to Django not
 reloading related models when non-relational fields change.

 I see a few ways forward within the migration system, with various pros
 and cons
 1. Reload all related models, regardless if the changed field is a
 relational field or referenced by a relational field. This will come with
 a huge run-time penalty that the delaying brought
 1. Try to catch these kind of database error or do some Python-level
 checking beforehand (`assert model1 is
 model2._meta.get_field("foo").related_model`). If we run into any issue
 there, reload all related models. Depending on how expensive that is, we
 could possibly do that after each migration operation and make the delayed
 model reloading based on that.
 1. Reload all models after each migrations is applied when the
 `ProjectState.is_delayed is False`. That would bring the benefit that it
 doesn't matter if one applies migrations individually or in a batch.
 1. Leave as-is; the first migration can be optimized anyway

 I haven't completely made up my mind about the "best" approach here. But
 ideally we'd get rid of model classes in migrations and solely work off of
 the `ModelState`. My last attempt got stuck do to the backwards
 compatibility in the `SchemaEditor`.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.1ca552449acab25e0233f0fc6ac1a0a6%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-14 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Hasan Ramezani):

 Simon, I checked the `TODO` in `AlterField.state_forwards`, here is my
 understanding.
 When I run the test in my the patch(my patch in
 [https://github.com/django/django/pull/12061 PR] + your patch in
 comment:7) it fails on alter operation in `0002_alter_user_id` at line
 `assert f.remote_field.model._meta.concrete_model._meta.apps is self.apps`

 {{{
 operations = [
 migrations.AlterField(
 model_name="user",
 name="id",
 field=models.CharField(default=uuid.uuid4, max_length=32,
 primary_key=True),
 ),
 ]
 }}}

 It fails because there is an operation in  `0001_initial` which run before
 the above mentioned migration:


 {{{
 migrations.AlterField(
 model_name="RRset",
 name="subname",
 field=models.CharField(blank=False, max_length=178),
 ),

 }}}

 this is an alter migration and the `AlterField.state_forwards` reloads  3
 models of 4 (User, Domain, RRest) and don't reload the Token model.
 If I reload the Token model this test will be passed.


 Should I change the function to find proper models to reload?
 Is it the right solution?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.f6d6a2eccd7b88ebe33423c3cf3a456b%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-13 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 Hasan, the changes in `django/db/migrations/state.py` from comment:7 are
 likely desired but the assertion in `Options` were more of a way to test
 which operation currently break this contract. From your submitted PR only
 three `migrations` tests were failing with these assertions including the
 one you added even on SQLite.

 Again, I suspect the cause is #27737 and the `TODO` in
 `AlterField.state_forwards`. The fact the rendering is delayed prevents
 `Token` from being refreshed adequately.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.90cc0548f9d27b3f4f05aeffb6a90ea2%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-13 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Hasan Ramezani):

 Sure, I can check it again.
 Just for confirmation, Should I fix assertaion failures based on Simon's
 patch?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.4667d2df4fe30fb84e5dbceb634c8894%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-13 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by felixxm):

 * needs_better_patch:  0 => 1


Comment:

 Simon, thanks for an investigation! I agree that we should try to address
 the main issue.

 Hasan, Can you take a look?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.4bc99e7fea09125483088745641c5011%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-12 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by felixxm):

 * cc: Markus Holtermann (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.0a101112e16f1565a4dbceb47b31538a%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-12 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 From giving a quick look at the current implementation it looks we're
 unfortunately sharing `__fake__` models bound to different `StateApps` for
 performance reason to work around the fact the schema editors are still
 operating from model classes instead of model states #29898.

 Since we'll likely won't be able to prevent this sharing from taking place
 in the short term for performance reasons we should investigate ''why''
 the models cache is not appropriately busted to avoid having two different
 `Options` instances for the `User` model. In this case it looks like the
 `Token` fake model is not reloaded while the `RRset`, `User`, and `Domain`
 ones are when the `AlterField(model_name="RRset")` is performed. That
 makes `Token.user` point to the ''stale'' `User` model and `Domain.owner`
 point to the new one.

 I highly suspect this ticket is related to #27737 and
 
[https://github.com/django/django/blob/b93a0e34d9b9b99d41103782b7e7aeabf47517e3/django/db/migrations/operations/fields.py#L231-L239
 this TODO] in `AlterField.state_forwards`.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.9f520c66ea9e6cf7262ea4456b5d7f8b%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-12 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 Hasan, the solution you proposed will likely address this issue but the
 fact multiple instances of `Options` for the same model classes are mixed
 in there is likely a symptom or a larger issue.

 I'm not sure of the origin of it but this can only happen when model from
 different `apps` boundaries are mixed together which should be disallowed
 as it will cause all sort of weird issues. What I mean by that is that
 there's likely ''something'' that creates models from a migration project
 state and have ''real'' models reference them or vice-versa. You can
 assert this doesn't happen by making sure each `model._meta.apps is
 self.apps`.

 In short I think this solution will only temporarily hide a much larger
 problem.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.cf01efbf92a3baa029bfbaaedd31c377%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-12 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Hasan Ramezani):

 * has_patch:  0 => 1


Comment:

 Postgres has this issue as well.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.60c3f4f4238fa2715a3407681c85%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-12 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+-
 Reporter:  Peter Thomassen  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Hasan Ramezani):

 * status:  new => assigned
 * owner:  nobody => Hasan Ramezani


Comment:

 I investigate it and think the problem comes from  this line:

 {{{
 
related_objects_graph[f.remote_field.model._meta.concrete_model._meta].append(f)
 }}}
 of
 
[https://github.com/django/django/blob/30359496a3f3d9af0b02afc334710f7e24c74f5b/django/db/models/options.py#L685
 django.db.models.options.Options._populate_directed_relation_graph()]:

 I added some log to check `related_objects_graph` in bothe case.
 When the migration runs by `./manage.py migrate desecapi 0001` and then
 `./manage.py migrate desecapi 0002` (correct case), here is the result:


 {{{
 defaultdict(, {:
 [,
 ], :
 []})
 }}}


 But, When the migration runs by `./manage.py migrate desecapi 0002` (case
 with problem),  here is the result:


 {{{
 defaultdict(, {:
 [], :
 [], : []})
 }}}

 From the result, it is obvious that in the first case we have the
 `` key with a value of type list with two items
 `[,
 ]`.
 which generate two alter commands:


 {{{
 ALTER TABLE `app1_domain` DROP FOREIGN KEY
 `app1_domain_owner_id_cc7262a2_fk_app1_user_id`
 ALTER TABLE `app1_token` DROP FOREIGN KEY
 `app1_token_user_id_5ea51092_fk_app1_user_id`
 }}}

 But in the second case, we have two keys ``  which one
 of them has a value of type list with one item
 `[]`
 and the other has a value of type list with one item
 `[]`.

 which generate just one alter command:

 {{{
 ALTER TABLE `app1_domain` DROP FOREIGN KEY
 `app1_domain_owner_id_cc7262a2_fk_app1_user_id`
 }}}

 Thus, the second foreign key `app1_token_user_id_5ea51092_fk_app1_user_id`
 still remains and cause the error.

 Any suggestions for a fix?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.69ff83d9846e7ce1d25fb5569bd3bf1b%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL.

2019-11-11 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+
 Reporter:  Peter Thomassen  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+
Changes (by Simon Charette):

 * cc: Simon Charette (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.c33ceec61b3eff176ea988a28350524b%40djangoproject.com.


Re: [Django] #30966: Migration crashes due to foreign key issue, depending on otherwise irrelevant order on MySQL. (was: Migration crashes due to foreign key issue, depending on otherwise irrelevant o

2019-11-11 Thread Django
#30966: Migration crashes due to foreign key issue, depending on otherwise
irrelevant order on MySQL.
-+
 Reporter:  Peter Thomassen  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migration mySQL  | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+
Changes (by felixxm):

 * keywords:  migration => migration mySQL
 * version:  2.2 => master
 * component:  Database layer (models, ORM) => Migrations
 * stage:  Unreviewed => Accepted


Comment:

 Thanks for this report, I confirmed this issue on MySQL. Scenario is
 really tricky, without the last operation i.e.
 {{{
 migrations.AlterField(
 model_name='rrset',
 name='subname',
 field=models.CharField(blank=False, max_length=178),
 ),
 }}}
 everything works fine. When we take it into account, migrations don't drop
 a `FOREIGN KEY` constraint from `desecapi_token`.

 Reproduced at 85efc14a2edac532df1a9ad4dd9b6d4a4dcf583e.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.67a0d436e7cc6493ad23b55f8fb752e5%40djangoproject.com.