File storage and save/commit=False
|Reported by:||shadfc||Owned by:|
|Has patch:||no||Needs documentation:||yes|
|Needs tests:||no||Patch needs improvement:||yes|
Background: I noticed this when playing with save_formset() on ModelAdmin in order to modify the data for a FileField on an inline model before it gets sent to S3 via a custom filestorage backend.
def save_formset(self, request, form, formset, change): instances = formset.save(commit=False) for instance in instances: if not instance.id: raise Exception('testing file creation') instance.save() formset.save_m2m()
Now, when saving a new inline instance which has a file field, the exception is raised before that instance gets save()'d. I kept noticing, however, that I would still get the full uploaded file with the proper name in my S3 account. So, I raised an exception in the _save() on my custom backend and walked it back to the formset.save(commit=False) call.
It seems to me that the commit=False should keep file writes from happening; it doesn't. Looking in django/db/models/fields/files.py, the save() takes a save argument but doesn't use it when dealing with the save call to the storage backend. If the argument was passed along to the save on the storage backend, then it could decide whether or not to actually write the file.
Change History (15)
comment:1 Changed 8 years ago by
|Patch needs improvement:||unset|
|Summary:||Filestorage backend interface for save() should accept save/commit argument → Filestorage backend doesn't get or use "save" argument|
comment:2 Changed 8 years ago by
|Summary:||Filestorage backend doesn't get or use "save" argument → Filestorage backend doesn't get "save" argument|
comment:3 Changed 8 years ago by
|Owner:||changed from nobody to shadfc|
|Status:||new → assigned|
|Summary:||Filestorage backend doesn't get "save" argument → File storage and save/commit=False|
|Triage Stage:||Unreviewed → Design decision needed|
comment:5 Changed 8 years ago by
|Triage Stage:||Design decision needed → Accepted|