[Django] #30691: Change uuid field to FK does not create dependency

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #30691: Change uuid field to FK does not create dependency

Django
#30691: Change uuid field to FK does not create dependency
-----------------------------------------+--------------------------
               Reporter:  Dart           |          Owner:  nobody
                   Type:  Bug            |         Status:  new
              Component:  Uncategorized  |        Version:  2.2
               Severity:  Normal         |       Keywords:  UUID, FK
           Triage Stage:  Unreviewed     |      Has patch:  0
    Needs documentation:  0              |    Needs tests:  0
Patch needs improvement:  0              |  Easy pickings:  0
                  UI/UX:  0              |
-----------------------------------------+--------------------------
 Hi! I am new in django community, so please help me, because i really dont
 know is it really "bug".

 I have a django project named "testproject" and two apps: testapp1,
 testapp2.

 It will be simpler to understand, with this example:

 # TestApp1(models.py):

 class App1(models.Model):
     id = models.UUIDField(primary_key=True, unique=True,
 default=uuid.uuid4, editable=False, verbose_name=_('identifier'))
     text = models.CharField(max_length=100, verbose_name=_('text'))
     another_app = models.UUIDField(null=True, blank=True,
 verbose_name=_('another app'))

 # TestApp2(models.py):

 class App2(models.Model):
     id = models.UUIDField(primary_key=True, unique=True,
 default=uuid.uuid4, editable=False, verbose_name=_('identifier'))
     text = models.CharField(max_length=100, verbose_name=_('text'))

 First model named "App1" has UUID field named "another_app":

 -  another_app = models.UUIDField(null=True, blank=True,
 verbose_name=_('another app'))

 After some time i change field from UUID to FK, like this:

 - another_app = models.ForeignKey(App2, null=True, blank=True,
 on_delete=models.SET_NULL, verbose_name=_('another app'))

 And as result i create new migration, but Migration class was unexpected
 result, because it does not create any "dependencies" for App2, because of
 FK.

 I think the correct solution will be create dependency for App2.

 This project use django version 2.2 and postgresql. Attach archive with
 sources. Project contains small test, after running him, you will get
 exception like this: ValueError: Related model 'testapp2.App2' cannot be
 resolved.

 So is it problem in django or maybe i dont understand something ?

 Here is my post in django users:
 https://groups.google.com/forum/#!searchin/django-
 users/Django$20bug$3A$20change$20uuid$20field$20to$20FK$20does$20not$20create$20dependency%7Csort:date
 /django-users/-h9LZxFomLU/yz-NLi1cDgAJ

 Regards, Viktor Lomakin

--
Ticket URL: <https://code.djangoproject.com/ticket/30691>
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/051.ad053e9acb592b842b53f9e892334802%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
#30691: Change uuid field to FK does not create dependency
-------------------------------+--------------------------------------
     Reporter:  Dart           |                    Owner:  nobody
         Type:  Bug            |                   Status:  new
    Component:  Uncategorized  |                  Version:  2.2
     Severity:  Normal         |               Resolution:
     Keywords:  UUID, FK       |             Triage Stage:  Unreviewed
    Has patch:  0              |      Needs documentation:  0
  Needs tests:  0              |  Patch needs improvement:  0
Easy pickings:  0              |                    UI/UX:  0
-------------------------------+--------------------------------------
Changes (by Dart):

 * Attachment "testproject.zip" added.

 Django project which show the bug

--
Ticket URL: <https://code.djangoproject.com/ticket/30691>
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.64c2bd93ac08b968a11569824da27032%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  nobody
         Type:  Bug         |                   Status:  new
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------
Changes (by Carlton Gibson):

 * version:  2.2 => master
 * component:  Uncategorized => Migrations
 * stage:  Unreviewed => Accepted


Comment:

 There's a lot of history in here, so it may turn out this is a duplicate,
 but I'll accept this for now, as there's certainly a bug.

 `makemigrations` generats this:


 {{{
     dependencies = [
         ('testapp1', '0002_app1_another_app'),
     ]

     operations = [
         migrations.AlterField(
             model_name='app1',
             name='another_app',
             field=models.ForeignKey(blank=True, null=True,
 on_delete=django.db.models.deletion.SET_NULL, to='testapp2.App2',
 verbose_name='another app'),
         ),
     ]
 }}}

 But that gives the `ValueError` as you say.

 Adjusting `dependencies` allows the test to pass:


 {{{
     dependencies = [
         ('testapp1', '0002_app1_another_app'),
         ('testapp2', '0001_initial'),
     ]
 }}}

 Thus, `AlterField` should pick this up.

 (Note for later: SQLite raised will required disabling constraint
 checking, ref #30023.)

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.ef8d84628790e8cdad0bc5dd4fafa695%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------
Changes (by Dart):

 * status:  new => assigned
 * owner:  nobody => Dart


--
Ticket URL: <https://code.djangoproject.com/ticket/30691#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.be648150a764cadd5caacb6fa41c501a%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------

Old description:

> Hi! I am new in django community, so please help me, because i really
> dont know is it really "bug".
>
> I have a django project named "testproject" and two apps: testapp1,
> testapp2.
>
> It will be simpler to understand, with this example:
>
> # TestApp1(models.py):
>
> class App1(models.Model):
>     id = models.UUIDField(primary_key=True, unique=True,
> default=uuid.uuid4, editable=False, verbose_name=_('identifier'))
>     text = models.CharField(max_length=100, verbose_name=_('text'))
>     another_app = models.UUIDField(null=True, blank=True,
> verbose_name=_('another app'))
>
> # TestApp2(models.py):
>
> class App2(models.Model):
>     id = models.UUIDField(primary_key=True, unique=True,
> default=uuid.uuid4, editable=False, verbose_name=_('identifier'))
>     text = models.CharField(max_length=100, verbose_name=_('text'))
>
> First model named "App1" has UUID field named "another_app":
>
> -  another_app = models.UUIDField(null=True, blank=True,
> verbose_name=_('another app'))
>
> After some time i change field from UUID to FK, like this:
>
> - another_app = models.ForeignKey(App2, null=True, blank=True,
> on_delete=models.SET_NULL, verbose_name=_('another app'))
>
> And as result i create new migration, but Migration class was unexpected
> result, because it does not create any "dependencies" for App2, because
> of FK.
>
> I think the correct solution will be create dependency for App2.
>
> This project use django version 2.2 and postgresql. Attach archive with
> sources. Project contains small test, after running him, you will get
> exception like this: ValueError: Related model 'testapp2.App2' cannot be
> resolved.
>
> So is it problem in django or maybe i dont understand something ?
>
> Here is my post in django users:
> https://groups.google.com/forum/#!searchin/django-
> users/Django$20bug$3A$20change$20uuid$20field$20to$20FK$20does$20not$20create$20dependency%7Csort:date
> /django-users/-h9LZxFomLU/yz-NLi1cDgAJ
>
> Regards, Viktor Lomakin
New description:

 Hi! I am new in django community, so please help me, because i really dont
 know is it really "bug".

 I have a django project named "testproject" and two apps: testapp1,
 testapp2.

 It will be simpler to understand, with this example:

 # TestApp1(models.py):

 {{{#!python
 class App1(models.Model):
     id = models.UUIDField(primary_key=True, unique=True,
 default=uuid.uuid4, editable=False, verbose_name=_('identifier'))
     text = models.CharField(max_length=100, verbose_name=_('text'))
     another_app = models.UUIDField(null=True, blank=True,
 verbose_name=_('another app'))
 }}}

 # TestApp2(models.py):

 {{{#!python
 class App2(models.Model):
     id = models.UUIDField(primary_key=True, unique=True,
 default=uuid.uuid4, editable=False, verbose_name=_('identifier'))
     text = models.CharField(max_length=100, verbose_name=_('text'))
 }}}

 First model named "App1" has UUID field named "another_app":

 -  `another_app = models.UUIDField(null=True, blank=True,
 verbose_name=_('another app'))`

 After some time i change field from UUID to FK, like this:

 - `another_app = models.ForeignKey(App2, null=True, blank=True,
 on_delete=models.SET_NULL, verbose_name=_('another app'))`

 And as result i create new migration, but Migration class was unexpected
 result, because it does not create any "dependencies" for App2, because of
 FK.

 I think the correct solution will be create dependency for App2.

 This project use django version 2.2 and postgresql. Attach archive with
 sources. Project contains small test, after running him, you will get
 exception like this: ValueError: Related model 'testapp2.App2' cannot be
 resolved.

 So is it problem in django or maybe i dont understand something ?

 Here is my post in django users:
 https://groups.google.com/forum/#!searchin/django-
 users/Django$20bug$3A$20change$20uuid$20field$20to$20FK$20does$20not$20create$20dependency%7Csort:date
 /django-users/-h9LZxFomLU/yz-NLi1cDgAJ

 Regards, Viktor Lomakin

--

Comment (by Simon Charette):

 That's probably a bug in
 `django.db.migrations.autodetector.MigrationAutodetector.generate_altered_fields`'s
 [https://github.com/django/django/blob/514efa3129792ec2abb2444f3e7aeb3f21a38386/django/db/migrations/autodetector.py#L966-L974
 call to add_operation] which doesn't pass any `dependencies` when
 `new_field.is_relation and not old_field.is_relation`. The dependencies
 can be retrieved by using
 `self._get_dependencies_for_foreign_key(new_field)`.

 We want to depend on the latest migration of the referenced app instead of
 the initial one though as the model we're referencing might have been
 altered since then but that's something the aforementioned method should
 handle.

 Tests should be added to `django/tests/migrations/test_autodetector.py`.

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.9a4d94059e7e2d16d1b5b388847e42d9%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------

Comment (by Simon Charette):

 Hello Dart,

 Are you still planing to work on this? Do you need any assistance
 submitting a patch?

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.7b0632252bf10a2298b601b6eb5d75c4%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------

Comment (by Dmitrij Strelnikov):

 I'm pretty sure your db change interfere with migration best practice.
 You are changing field type, which have to be done in two steps.
 And your migrations are not telling django what should be done with data
 in uuid field contains when migrate.

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.d4d2679a2940f42527d9900143958593%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------

Comment (by Dart):

 Replying to [comment:4 Simon Charette]:
 > Hello Dart,
 >
 > Are you still planing to work on this? Do you need any assistance
 submitting a patch?
 Yes i'm work with this patch and planning to send this. Yes, it will be
 good, if you help me.

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.c087a22303c98d218a7cb9b1dc727478%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------

Comment (by Dart):

 Replying to [comment:4 Simon Charette]:
 > Hello Dart,
 >
 > Are you still planing to work on this? Do you need any assistance
 submitting a patch?

 Hi! Today i will send a patch. I found a problem and resolve it.

 Regards

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.59d25d4d8626b6b1c1c9ddb574aeaa98%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  0           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------

Comment (by Dart):

 Hi! Send pull request, please check it. And please can you help me where
 to put docs.

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#comment:8>
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.1ed1fefd8759bccfce900591143c89ea%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             Triage Stage:  Accepted
    Has patch:  1           |      Needs documentation:  0
  Needs tests:  0           |  Patch needs improvement:  0
Easy pickings:  0           |                    UI/UX:  0
----------------------------+------------------------------------
Changes (by Dart):

 * has_patch:  0 => 1


Comment:

 https://github.com/django/django/pull/11740

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#comment:9>
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.0cc04c59bc2f8051ce46842465ff68ca%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+---------------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  assigned
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:
     Keywords:  UUID, FK    |             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 Simon Charette):

 * stage:  Accepted => Ready for checkin


Comment:

 Hey Dart, thanks for submitting a PR!

 Since this is a bug fix it shouldn't not require any docs. I've marked the
 patch RFC since only trivial changes need to be made before the patch is
 merged which is something committers can take care of.

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#comment:10>
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.11ded545e922dc129b8a11839c4250f5%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #30691: Change uuid field to FK does not create dependency

Django
In reply to this post by Django
#30691: Change uuid field to FK does not create dependency
----------------------------+---------------------------------------------
     Reporter:  Dart        |                    Owner:  Dart
         Type:  Bug         |                   Status:  closed
    Component:  Migrations  |                  Version:  master
     Severity:  Normal      |               Resolution:  fixed
     Keywords:  UUID, FK    |             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 Mariusz Felisiak <felisiak.mariusz@…>):

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


Comment:

 In [changeset:"5931d2e96ae94b204d146b7f751e0e804da74953" 5931d2e9]:
 {{{
 #!CommitTicketReference repository=""
 revision="5931d2e96ae94b204d146b7f751e0e804da74953"
 Fixed #30691 -- Made migrations autodetector find dependencies for foreign
 keys altering.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30691#comment:11>
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.6fedbbe44dac6628cbc6869537541887%40djangoproject.com.