Opened 12 years ago
Closed 12 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 , 12 years ago
| Severity: | Normal → Release blocker | 
|---|---|
| Triage Stage: | Unreviewed → Accepted | 
| Version: | 1.7-beta-1 → master | 
comment:2 by , 12 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 , 12 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 , 12 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.