[Django] #32689: Infinite AlterField Migrations due to default callable object missmatch
Django
-------------------------------------+-------------------------------------
Reporter: Samuel | Owner: nobody
Bishop |
Type: Bug | Status: new
Component: | Version: 3.2
Migrations | Keywords: infinite,
Severity: Normal | makemigrations, migrations,
Triage Stage: | callable, default, field
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`makemigration` will generate infinite migrations if the callable path is
miss-matched such that you end up with two discreet callable objects (I'm
still trying to identify where exactly the migration framework is
generating this missmatch) then the same migration is generated every time
you run the command which results in no actual change to the database.
In my case it's from a custom field library being used as a primary key.
I was importing `from ulid.api import new as new_ulid` and setting
`default=new_ulid`, but unless I swap to `import ulid.api.api` and set
`default=ulid.api.api.Api.new` I just get a new identical migration
altering the field's default value to `ulid.api.api.Api.new` every time i
run `makemigrations`
I would have expected either a clear warning detailing why my choice of
callable object for the `default=` will cause issues, or I would have
expected `makemigrations` to not mutate my callable like this.
--
Ticket URL: <https://code.djangoproject.com/ticket/32689>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: infinite, | Triage Stage:
makemigrations, migrations, | Unreviewed
callable, default, field |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> `makemigration` will generate infinite migrations if the callable path is
> miss-matched such that you end up with two discreet callable objects (I'm
> still trying to identify where exactly the migration framework is
> generating this missmatch) then the same migration is generated every
> time you run the command which results in no actual change to the
> database.
>
> In my case it's from a custom field library being used as a primary key.
>
> I was importing `from ulid.api import new as new_ulid` and setting
> `default=new_ulid`, but unless I swap to `import ulid.api.api` and set
> `default=ulid.api.api.Api.new` I just get a new identical migration
> altering the field's default value to `ulid.api.api.Api.new` every time i
> run `makemigrations`
>
> I would have expected either a clear warning detailing why my choice of
> callable object for the `default=` will cause issues, or I would have
> expected `makemigrations` to not mutate my callable like this.
New description:
`makemigration` will generate infinite migrations if the callable path is
miss-matched such that you end up with two discreet callable objects (I'm
still trying to identify where exactly the migration framework is
generating this missmatch, I can see the difference stepping through
`MigrationAutodetector.generate_altered_fields` in the debugger but I'm
not sure if the root cause is inside `generate_altered_fields`, another
method on `MigrationAutodetector` or somewhere else entirely...) then the
same migration is generated every time you run the command which results
in no actual change to the database.
In my case it's from a custom field library being used as a primary key.
I was importing `from ulid.api import new as new_ulid` and setting
`default=new_ulid`, but unless I swap to `import ulid.api.api` and set
`default=ulid.api.api.Api.new` I just get a new identical migration
altering the field's default value to `ulid.api.api.Api.new` every time i
run `makemigrations`
I would have expected either a clear warning detailing why my choice of
callable object for the `default=` will cause issues, or I would have
expected `makemigrations` to not mutate my callable like this.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32689#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: infinite, | Triage Stage:
makemigrations, migrations, | Unreviewed
callable, default, field |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> `makemigration` will generate infinite migrations if the callable path is
> miss-matched such that you end up with two discreet callable objects (I'm
> still trying to identify where exactly the migration framework is
> generating this missmatch, I can see the difference stepping through
> `MigrationAutodetector.generate_altered_fields` in the debugger but I'm
> not sure if the root cause is inside `generate_altered_fields`, another
> method on `MigrationAutodetector` or somewhere else entirely...) then the
> same migration is generated every time you run the command which results
> in no actual change to the database.
>
> In my case it's from a custom field library being used as a primary key.
>
> I was importing `from ulid.api import new as new_ulid` and setting
> `default=new_ulid`, but unless I swap to `import ulid.api.api` and set
> `default=ulid.api.api.Api.new` I just get a new identical migration
> altering the field's default value to `ulid.api.api.Api.new` every time i
> run `makemigrations`
>
> I would have expected either a clear warning detailing why my choice of
> callable object for the `default=` will cause issues, or I would have
> expected `makemigrations` to not mutate my callable like this.
New description:
`makemigration` will generate infinite migrations if the callable path is
miss-matched such that you end up with two discreet callable objects (I'm
still trying to identify where exactly the migration framework is
generating this missmatch, I can see the difference stepping through
`MigrationAutodetector.generate_altered_fields` in the debugger but I'm
not sure if the root cause is inside `generate_altered_fields`, another
method on `MigrationAutodetector` or somewhere else entirely...) then the
same migration is generated every time you run the command which results
in no actual change to the database.
In my case it's from a custom field library being used as a primary key.
I was importing `from ulid.api import new as new_ulid` and setting
`default=new_ulid`, but unless I swap to `import ulid.api.api` and set
`default=ulid.api.api.Api.new` I just get a new identical migration
altering the field's default value to `ulid.api.api.Api.new` every time i
run `makemigrations`
I would have expected either a clear warning detailing why my choice of
callable object for the `default=` will cause issues, or I would have
expected `makemigrations` to not mutate my callable like this.
Root cause: Turns out this is a serialisation fault. The migration
serialiser doesn't throw any kind of error or warning and just mutates the
callable instead of preventing me from using an invalid default callable
that the migration framework wont be able to serialise. A simple wrapper
function added to the custom field library solves the matter entirely,
however the underlying issue of the migration framework behaviour is worth
looking at improving.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32689#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: infinite, | Triage Stage:
makemigrations, migrations, | Unreviewed
callable, default, field |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> The migration serialiser doesn't throw any kind of error or warning and
just mutates the callable instead of preventing me from using an invalid
default callable that the migration framework wont be able to serialise. A
simple wrapper function added to the custom field library solves the
matter entirely, however the underlying issue of the migration framework
behaviour is worth looking at improving.
Can you add more details about this callable (an example)? or maybe a
sample project.
--
Ticket URL: <https://code.djangoproject.com/ticket/32689#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: infinite, | Triage Stage:
makemigrations, migrations, | Unreviewed
callable, default, field |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Pablo):
I was able to "reproduce" using sqlite as a DB backend with the following
model:
{{{
from ulid.api import new as new_ulid
from django.db import models
# Create your models here.
class SampleModel(models.Model):
ulid = models.CharField(
max_length=255,
null=False,
default=new_ulid
)
def __str__(self):
return str(self.ulid)
}}}
It's important to mention that the created migration is useless. It cannot
be applied due to an exception calling new_ulid: TypeError: new() missing
1 required positional argument: 'self'.
Might be related to the fact that new() is an instance method:
https://github.com/ahawker/ulid/blob/96bdb1daad7ce96f6db8c91ac0410b66d2e1c4c1/ulid/api/api.py#L51
If you create a proxy function, the migration is created and applied
correctly:
{{{
from ulid.api import new as new_ulid
from django.db import models
def ulid_proxy():
return new_ulid()
# Create your models here.
class SampleModel(models.Model):
ulid = models.CharField(
max_length=255,
null=False,
default=ulid_proxy
)
def __str__(self):
return str(self.ulid)
}}}
So I'm not sure if this is a bug related on how migrations are created or
if this is has something to do with how new() is implemented in ulid.api
--
Ticket URL: <https://code.djangoproject.com/ticket/32689#comment:4>
Django
-------------------------------------+-------------------------------------
Component: Migrations | Version: 3.2
Severity: Normal | Resolution: invalid
Keywords: infinite, | Triage Stage:
makemigrations, migrations, | Unreviewed
callable, default, field |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
I don't think there is anything we can improve in Django. `new` is a
different object each time it's imported, so Django properly detects this
as change.
--
Ticket URL: <https://code.djangoproject.com/ticket/32689#comment:5>