Opened 13 years ago
Last modified 9 years ago
#17224 new Cleanup/optimization
determine and document the use of default option in context of FileField
Reported by: | Preston Holmes | Owned by: | nobody |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Normal | 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
Currently the 'default' option in the definition of a "FileField" just gets passed on to Field where it is just stored in the database as a raw string.
There are several other potentially viable things FileField could do, including checking to see if it is a valid path to a file, and then copy that to "upload_to" and make that the field's contents.
Either way, whatever is the "proper" action for default, even if it remains unchanged, it should be documented in the FileField docs.
Change History (4)
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 10 years ago
#24823 complained about FileField
with callable default raising an error with forms. We should reopen that ticket if we decide it should be supported.
comment:3 by , 9 years ago
Hello,
I am the reporter of the bug #24823.
Just my 2 cents on this problem.
I know that the documentation of FileField is explicit ("A file-upload field.") but I think that in certain use cases the semantic of this field has changed to be a field simply referencing a file, whatever its origin.
This is especially true for the ImageField that add some convenient attributes to manipulate image. This semantic shift is even bigger in packages that add fields that derive form ImageField or FileField, like the packages that helps in the manipulation of thumbnails.
In this case the duplication of the file pointed by the value makes not always a lot of sense. For example, one may need to update the content of the "default" field for all instances pointing to it. If the file has been duplicated, this otherwise very simple operation can be now quite difficult.
For the default value of the FileField, I would recommend to have the same behavior as the char field (plus the initialization of the instance.file
property). This is indeed the current behavior for string default value and since there is no warning in the documentation, this should be the expected behavior for callable default values.
If the file duplication behavior described above is needed, this can be done in a callable and be explained in the documentation.
However, for a callable default value, the type and value of the expected return value must be precisely defined (in the documentation and perhaps in the code with some type check and an explicit error message).
To put things in context, I stumbled on this "bug" by migrating our application to Django 1.8. Some of our image field has a default value defined in the settings; therefore its value depends on the app deployment parameters. When examining the migration files generated by Django, I saw that the default value for these fields was the resolved string value of the setting, which is not correct for all deployment. The solution was to change the default value to be a callable that just returns the setting value. It seemed to work just fine, until we tried to create the object using a form (like in the admin) where an exception is raised as described in bug #24823.
Regards,
ymh
comment:4 by , 9 years ago
Note that the issue about model field defaults that refer to settings is tracked in #24648. I'm not convinced that moving to a callable is the best solution here, but that discussion is for that ticket.
Duplicating the file pointed by the value of "default" would be the most logical behavior, but it's going to be a bad idea in most cases: since files are never deleted, if would cause lots of useless duplication.
Therefore, it may be preferable to ensure that
FileField
andImageField
ignore this argument, and document that behavior.