[Django] #29125: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29125: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.

Django
#29125: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.
-------------------------------------+-------------------------------------
               Reporter:  Harro      |          Owner:  nobody
                   Type:  Bug        |         Status:  new
              Component:  Database   |        Version:  2.0
  layer (models, ORM)                |
               Severity:  Release    |       Keywords:  has_test
  blocker                            |
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 Here is a branch with a randomly failing test that proves this:
 https://github.com/hvdklauw/django/blob/bug/q_destruct/tests/queries/test_q.py#L63

 The biggest issue is that now makemigrations is detecting changes since we
 upgraded to django 2.0 (only with python 3.5) that aren't changes at all,
 just reordered kwargs on the Q objects in out limit_choices_to on some
 foreignkeys.

 This also means we randomly can't commit/release because we have pre-
 commit hooks and CI that runs ''makemigrations --check --dryrun''

 Upgrading to python 3.6 would fix it (because of ordered kwargs) but as
 Ubuntu 16.04 is still the latest LTS, python 3.5 is what we are stuck
 with.

--
Ticket URL: <https://code.djangoproject.com/ticket/29125>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/051.a5cc1ed39744b658671fee700db5b409%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29125: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.

Django
#29125: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.
-------------------------------------+-------------------------------------
     Reporter:  Harro                |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:
     Keywords:  has_test             |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Description changed by Harro:

Old description:

> Here is a branch with a randomly failing test that proves this:
> https://github.com/hvdklauw/django/blob/bug/q_destruct/tests/queries/test_q.py#L63
>
> The biggest issue is that now makemigrations is detecting changes since
> we upgraded to django 2.0 (only with python 3.5) that aren't changes at
> all, just reordered kwargs on the Q objects in out limit_choices_to on
> some foreignkeys.
>
> This also means we randomly can't commit/release because we have pre-
> commit hooks and CI that runs ''makemigrations --check --dryrun''
>
> Upgrading to python 3.6 would fix it (because of ordered kwargs) but as
> Ubuntu 16.04 is still the latest LTS, python 3.5 is what we are stuck
> with.
New description:

 Here is a branch with a randomly failing test that proves this:
 https://github.com/hvdklauw/django/blob/bug/q_destruct/tests/queries/test_q.py#L63

 The biggest issue is that now makemigrations is detecting changes since we
 upgraded to django 2.0 (only with python 3.5) that aren't changes at all,
 just reordered kwargs on the Q objects in out limit_choices_to on some
 foreignkeys.

 This also means we randomly can't commit/release because we have pre-
 commit hooks and CI that runs ''makemigrations --check --dryrun''

 Upgrading to python 3.6 would fix it (because of ordered kwargs) but as
 Ubuntu 16.04 is still the latest LTS, python 3.5 is what we are stuck
 with.

 This is the commit that broke it:
 https://github.com/django/django/commit/508b5debfb16843a8443ebac82c1fb91f15da687

--

--
Ticket URL: <https://code.djangoproject.com/ticket/29125#comment:1>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.206b2000b6ffbf7847d666a9f81d585f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29125: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.

Django
In reply to this post by Django
#29125: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.
-------------------------------------+-------------------------------------
     Reporter:  Harro                |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:
     Keywords:  has_test             |             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):

 * stage:  Unreviewed => Accepted


Comment:

 Apart from preserving the `kwargs` nature of some children I'd argue that
 `Q.deconstruct` should also use `django.utils.models.Q` as path instead of
 `django.db.models.query_utils.Q` like we do with
 `django.db.models.fields.*` and should avoid adding `_connector` if it's
 `'AND'` and `_negated` if it's `False` to `kwargs` because these are the
 default values.

--
Ticket URL: <https://code.djangoproject.com/ticket/29125#comment:2>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.3dc82a93007d06d387bda3a3df23acc6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs (was: BUG: Q object deconstruct is inconsistent when passing multiple kwargs.)

Django
In reply to this post by Django
#29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs
-------------------------------------+-------------------------------------
     Reporter:  Harro                |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               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 Tim Graham):

 * keywords:  has_test =>
 * has_patch:  0 => 1


Comment:

 [https://github.com/django/django/pull/9694 PR]

 I don't understand why `{'_connector': 'AND'}` should be omitted but I
 implemented it.

--
Ticket URL: <https://code.djangoproject.com/ticket/29125#comment:3>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.914a2e683a6c1596d56bc99af3c39d00%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs

Django
In reply to this post by Django
#29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs
-------------------------------------+-------------------------------------
     Reporter:  Harro                |                    Owner:  nobody
         Type:  Bug                  |                   Status:  closed
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:  fixed
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

 In [changeset:"9ba3df82402e7e23b353da20aea6894935241ef9" 9ba3df8]:
 {{{
 #!CommitTicketReference repository=""
 revision="9ba3df82402e7e23b353da20aea6894935241ef9"
 Refs #29125 -- Made Q.deconstruct() omit 'query_utils' in the path and
 _connector='AND' since it's a default value.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29125#comment:5>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.28775795a6a87837002e5119e33bb9d9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs

Django
In reply to this post by Django
#29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs
-------------------------------------+-------------------------------------
     Reporter:  Harro                |                    Owner:  nobody
         Type:  Bug                  |                   Status:  closed
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:  fixed
     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 Tim Graham <timograham@…>):

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


Comment:

 In [changeset:"b95c49c954e3b75678bb258e9fb2ec30d0d960bb" b95c49c9]:
 {{{
 #!CommitTicketReference repository=""
 revision="b95c49c954e3b75678bb258e9fb2ec30d0d960bb"
 Fixed #29125 -- Made Q.deconstruct() deterministic with multiple keyword
 arguments.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29125#comment:4>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.471c3e0a392908de1e2ace8a6e623eff%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs

Django
In reply to this post by Django
#29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs
-------------------------------------+-------------------------------------
     Reporter:  Harro                |                    Owner:  nobody
         Type:  Bug                  |                   Status:  closed
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:  fixed
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

 In [changeset:"aeb35548dc3a669aebec45f8f51a9cd3fbfc8801" aeb35548]:
 {{{
 #!CommitTicketReference repository=""
 revision="aeb35548dc3a669aebec45f8f51a9cd3fbfc8801"
 [2.0.x] Fixed #29125 -- Made Q.deconstruct() deterministic with multiple
 keyword arguments.

 Backport of b95c49c954e3b75678bb258e9fb2ec30d0d960bb from master
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29125#comment:6>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.485caf7f70e852743e33f47dfe9ae055%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs

Django
In reply to this post by Django
#29125: Q.deconstruct() is nondeterministic if the Q has multiple kwargs
-------------------------------------+-------------------------------------
     Reporter:  Harro                |                    Owner:  nobody
         Type:  Bug                  |                   Status:  closed
    Component:  Database layer       |                  Version:  2.0
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:  fixed
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

 In [changeset:"fd18345e10b9e5c9713c606509feaf18e57178e2" fd18345e]:
 {{{
 #!CommitTicketReference repository=""
 revision="fd18345e10b9e5c9713c606509feaf18e57178e2"
 [2.0.x] Refs #29125 -- Made Q.deconstruct() omit 'query_utils' in the path
 and _connector='AND' since it's a default value.

 Backport of 9ba3df82402e7e23b353da20aea6894935241ef9 from master
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29125#comment:7>
Django <https://code.djangoproject.com/>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.b8eb332be387fa2422d0de1cc03d9a48%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.