Opened 10 years ago
Last modified 5 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 |
Pull Requests: | How to create a pull request | ||
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'.
According to the ticket's flags, the next step(s) to move this issue forward are:
- To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is:
[https://github.com/django/django/pull/#### PR]
.
Change History (8)
comment:1 by , 10 years ago
Cc: | added |
---|---|
Summary: | Runtime attributes on fields may confuse migrations → Model fields that reference settings that differ between dev and prod trigger the autodetector |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 10 years ago
Cc: | added |
---|
comment:3 by , 10 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 , 9 years ago
Cc: | added |
---|
comment:5 by , 9 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 , 9 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 , 8 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 , 5 years ago
Cc: | added |
---|
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.