Opened 9 years ago

Last modified 4 years ago

#24648 new Cleanup/optimization

Model fields that reference settings that differ between dev and prod trigger the autodetector

Reported by: Christophe Pettus Owned by: nobody
Component: Migrations Version: 1.8
Severity: Normal Keywords:
Cc: Andrew Godwin, Markus Holtermann, ben@…, Alex Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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'.

Change History (8)

comment:1 by Tim Graham, 9 years ago

Cc: Andrew Godwin added
Summary: Runtime attributes on fields may confuse migrationsModel fields that reference settings that differ between dev and prod trigger the autodetector
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

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.

comment:2 by Markus Holtermann, 9 years ago

Cc: Markus Holtermann added

comment:3 by Tim Graham, 9 years ago

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.

comment:4 by Ben Kornrath, 9 years ago

Cc: ben@… added

comment:5 by Markus Holtermann, 8 years ago

For the time being and while I'm thinking of a proper solution, can you try this, please.

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)

comment:6 by Markus Holtermann, 8 years ago

I may point out that above is not a public API but is used by e.g. the swappable user model.

comment:7 by Tai Lee, 7 years ago

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.

comment:8 by Alex, 4 years ago

Cc: Alex added
Note: See TracTickets for help on using tickets.
Back to Top