Opened 3 years ago

Last modified 6 months 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@… 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 (7)

comment:1 Changed 3 years ago by Tim Graham

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 Changed 3 years ago by Markus Holtermann

Cc: Markus Holtermann added

comment:3 Changed 3 years ago by Tim Graham

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 Changed 2 years ago by Ben Kornrath

Cc: ben@… added

comment:5 Changed 20 months ago by Markus Holtermann

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 Changed 20 months ago by Markus Holtermann

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

comment:7 Changed 6 months ago 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.

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