[Django] #24648: Runtime attributes on fields may confuse migrations
Django
-------------------------------+--------------------
Reporter: Xof | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
If a model has an attribute whose value changes from development to
deployment due to a runtime dependency, this can cause migrations to think
the code has changed. (Whether it has or not is a philosophical question,
I suppose, but it's not a different set of text.)
Example:
In settings.py:
{{{
UPLOAD_ROOT = os.path.normpath(os.path.join(PROJECT_DIR, '..', '..',
'uploads'))
UPLOAD_URL = '/content/'
}}}
In the model:
{{{
upload_storage = FileSystemStorage(location=UPLOAD_ROOT,
base_url=UPLOAD_URL)
...
class Thingie(models.Model):
image = models.ImageField(upload_to='/%Y/%m/%d',
storage=upload_storage, null=True, default=None)
}}}
The migrations will consider Thingie to have changed from dev to
deployment based on the change of 'upload_storage'.
--
Ticket URL: <https://code.djangoproject.com/ticket/24648>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
trigger the autodetector
--------------------------------------+------------------------------------
Reporter: Xof | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: andrewgodwin (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
One idea (not sure if it would cause problems) is to modify the migration
file to replace the literal value `'/content/'` with a reference to the
setting, `settings.UPLOAD_URL`. If so, we could document this.
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:1>
Django
trigger the autodetector
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* cc: MarkusH (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:2>
Django
trigger the autodetector
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
[https://github.com/django/django/pull/4605 PR] has been proposed to allow
the `FileField` `storage` argument to take a callable in order to work
around this issue. It doesn't seem ideal to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:3>
Django
trigger the autodetector
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by benkonrath):
* cc: ben@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:4>
Django
trigger the autodetector
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
For the time being and while I'm thinking of a proper solution, can you
try this, please.
{{{#!python
from django.db.migrations.writer import SettingsReference
upload_root_reference = SettingsReference(UPLOAD_ROOT, 'UPLOAD_ROOT')
upload_storage = FileSystemStorage(location=upload_root_reference,
base_url=upload_root_reference)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:5>
Django
trigger the autodetector
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I may point out that above is not a public API but is used by e.g. the
swappable user model.
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:6>
Django
trigger the autodetector
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tai Lee):
We've encountered issues with migrations storing serialized settings at
the time the migration was created. We've always fixed them in our
projects and suggested to 3rd party projects that they simple edit the
resulting migration to import and use the setting directly instead of its
serialized value. No need for callables.
I suppose something like `SettingsReference` might allow the auto
migration to detect and properly reference the setting automatically, but
it seems like an *extremely* easy mistake to forget to do this in advance
and to end up with serialized migrations where the fix still requires
manually editing the migration, at which point the use of
`SettingsReference` is redundant.
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:7>
Django
trigger the autodetector
--------------------------------------+------------------------------------
Reporter: Christophe Pettus | Owner: nobody
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Alex):
* cc: Alex (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24648#comment:8>