Opened 8 years ago

Closed 3 years ago

Last modified 3 years ago

#8918 closed New feature (fixed)

FileField upload_to should not be required for custom backends

Reported by: Leah Culver Owned by: nobody
Component: File uploads/storage Version: master
Severity: Normal Keywords: FileField, files, upload, S3, upload_to
Cc: physicsnick@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

For some file storage backends, the upload_to could be unnecessary. For example, I have an S3 backend and don't need any keys or prefixes for my files.

Currently an error is thrown if upload_to is missing or empty: "file": FileFields require an "upload_to" attribute.

Could upload_to be optional if the storage backend is custom?

Attachments (2)

file.patch (4.1 KB) - added by dc 7 years ago.
Makes upload_to optional
FileField_upload_to_optional.diff (5.9 KB) - added by Vajrasky Kok 3 years ago.
The patch for master (1.7), making the upload_to optional

Download all attachments as: .zip

Change History (22)

comment:1 Changed 8 years ago by Leah Culver

Component: UncategorizedFile uploads/storage
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 8 years ago by Leah Culver

In a strange twist, if you define a function for upload_to, the function can return the empty string. This should at least be documented? If the function returns None the filename is "Nonefilename.txt"... heh.

comment:3 Changed 8 years ago by anonymous

Version: SVN

comment:4 Changed 8 years ago by Jacob

milestone: 1.1
Triage Stage: UnreviewedAccepted

There probably needs to be some flag you can set on the storage backend to indicate weather upload_to is required or not.

Changed 7 years ago by dc

Attachment: file.patch added

Makes upload_to optional

comment:5 Changed 7 years ago by dc

Has patch: set

upload_to can be optional - all infrastructure is here, actually only check in validation.py makes it required.

comment:6 Changed 7 years ago by Jacob

milestone: 1.11.2

Punting from 1.1: this is annoying, but there's an easy workaround (just set upload_to = "dummy" or something).

comment:7 Changed 7 years ago by physicsnick

Cc: physicsnick@… added

comment:8 Changed 7 years ago by Jacob

Triage Stage: AcceptedReady for checkin

comment:9 Changed 7 years ago by James Bennett

Triage Stage: Ready for checkinDesign decision needed

Bumping this off the "ready for checkin" stage following discussion with Jacob of some issues with the proposed solution.

In a nutshell, making upload_to optional makes code portability difficult; application code shouldn't differ based on the (deployment-specific) storage backend in use.

comment:10 Changed 7 years ago by Russell Keith-Magee

milestone: 1.21.3

Not critical for 1.2.

comment:11 Changed 6 years ago by Adam Nelson

I'm not sure if Django should stop the user from doing this, but dedicating an entire bucket to a FileField seems like a bad idea. Setting upload_to to any string creates a pseudo-folder on Amazon S3 at no cost except for extra characters in the URL.

comment:12 Changed 5 years ago by Luke Plant

Severity: Normal
Type: New feature

comment:13 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

comment:11 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:12 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:13 Changed 3 years ago by Jacob

Triage Stage: Design decision neededAccepted

Despite the portability issues, I still think upload_to should be optional.

comment:14 Changed 3 years ago by Tim Graham

Easy pickings: set
Patch needs improvement: set

Patch needs to be updated to apply cleanly and the change should be mentioned in the release notes (docs/releases/1.7.txt). These changes should be suitable for a first-time committer.

Changed 3 years ago by Vajrasky Kok

The patch for master (1.7), making the upload_to optional

comment:15 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 945e033a6964c8c83c1c5ce5f188baf41a7a7701:

Fixed #8918 -- Made FileField.upload_to optional.

Thanks leahculver for the suggestion and dc and vajrasky for work
on the patch.

comment:16 Changed 3 years ago by anonymous

Amazing. Thanks everyone!!

comment:17 Changed 3 years ago by Ramiro Morales <ramiro@…>

In 8ab5f1fe4750105dae734764baed54bd4d22792b:

Close file after tests added in 945e033a69.

Avoids failures on Windows. Refs #8918.

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