Opened 4 years ago

Last modified 3 months ago

#17224 new Cleanup/optimization

determine and document the use of default option in context of FileField

Reported by: ptone Owned by: nobody
Component: File uploads/storage Version: master
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 Changed 4 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

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 and ImageField ignore this argument, and document that behavior.

comment:2 Changed 4 months ago by timgraham

#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 Changed 3 months ago by ymph

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 Changed 3 months ago by timgraham

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.

Note: See TracTickets for help on using tickets.
Back to Top