Code

Opened 6 years ago

Closed 9 months ago

Last modified 9 months ago

#8918 closed New feature (fixed)

FileField upload_to should not be required for custom backends

Reported by: leahculver 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 5 years ago.
Makes upload_to optional
FileField_upload_to_optional.diff (5.9 KB) - added by vajrasky 9 months ago.
The patch for master (1.7), making the upload_to optional

Download all attachments as: .zip

Change History (22)

comment:1 Changed 6 years ago by leahculver

  • Component changed from Uncategorized to File uploads/storage
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by leahculver

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 6 years ago by anonymous

  • Version set to SVN

comment:4 Changed 5 years ago by jacob

  • milestone set to 1.1
  • Triage Stage changed from Unreviewed to Accepted

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

Changed 5 years ago by dc

Makes upload_to optional

comment:5 Changed 5 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 5 years ago by jacob

  • milestone changed from 1.1 to 1.2

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

comment:7 Changed 5 years ago by physicsnick

  • Cc physicsnick@… added

comment:8 Changed 4 years ago by jacob

  • Triage Stage changed from Accepted to Ready for checkin

comment:9 Changed 4 years ago by ubernostrum

  • Triage Stage changed from Ready for checkin to Design 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 4 years ago by russellm

  • milestone changed from 1.2 to 1.3

Not critical for 1.2.

comment:11 Changed 4 years ago by adamnelson

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 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:13 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:11 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:12 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:13 Changed 15 months ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

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

comment:14 Changed 9 months ago by timo

  • 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 9 months ago by vajrasky

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

comment:15 Changed 9 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

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 9 months ago by anonymous

Amazing. Thanks everyone!!

comment:17 Changed 9 months ago by Ramiro Morales <ramiro@…>

In 8ab5f1fe4750105dae734764baed54bd4d22792b:

Close file after tests added in 945e033a69.

Avoids failures on Windows. Refs #8918.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.