Opened 11 years ago
Closed 11 years ago
#22373 closed Bug (duplicate)
Django Migrations keeps detecting changes to FileField using custom storage
Reported by: | Owned by: | ||
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've a FileField that uses a custom storage backend, which is a subclass of FileSystemStorage. After adding a deconstruct function to my custom storage backend, Django's migration framework keeps detecting that the FileField has changed ad infinitum.
from django.core.files.storage import FileSystemStorage class MediaStorage(FileSystemStorage): def deconstruct(self): return ('foo.media.storage.MediaStorage', [], {})
The migration file shows:
migrations.AlterField( model_name='file', name='file', field=models.FileField(storage=foo.media.storage.MediaStorage(), upload_to='foo_media/', max_length=255, verbose_name=u'file'), ),
Note: This bug is related to #22337, but not the same.
Change History (5)
comment:1 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | 1.7-beta-1 → master |
comment:2 by , 11 years ago
Are the details of the storage actually used in the database? If not then I would argue that they have no place in the migration. I brought this up on several other related tickets (#22351 and #22436). If you make my_module.MyCustomStorage
part of the migration and then change the name or implementation details of it then the past migrations will be broken. My solution in #22426 is to have Field.deconstruct()
replace functions for upload_to
and other similar kwargs with a function that raises NotImplementedError when called. It seems to work (I'm very new at this, so please be brutal in your criticism), and I think a similar approach would work here. There's a link to my git commit in the last comment on #22426. Any thoughts?
comment:3 by , 11 years ago
@chriscauley: No, the details of the storage are not related to the database in any way. So if I can keep this out of the migration i'll be fine. I'll try your "workaround" of raising a NotImplementedException.
comment:4 by , 11 years ago
You should also consider just removing the storage kwarg in FileField.deconstruct()
. Since it's an optional argument this may make more sense. upload_to
is required this is not an option in the other tickets. Then again, if a migration somehow touches a storage it may be better to raise NotImplementedError
.
Hi,
It seems that the autodetector is comparing the instances of the storage class instead of their deconstructed versions.
Since the storage class doesn't implement
__eq__
, this means that the autodetector wrongly assumes you're using a new object and therefore create a new migration.Not sure what the best course of action is, but this is definitely a bug (I'll also mark it as a release blocker).
Thanks.