Opened 7 months ago

Last modified 7 months ago

#28144 assigned New feature

Add allow_overwrite kwarg to FileSystemStorage._save

Reported by: Jon Prindiville Owned by: Jon Prindiville
Component: File uploads/storage Version: 1.11
Severity: Normal Keywords: file storage overwrite
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Jon Prindiville)

I want to be able to construct a filesystem storage backend that will let me overwrite files while still leveraging FileSystemStorage.

This set of changes (https://github.com/django/django/compare/master...jonprindiville:ticket_28144) would enable that without changing any of the default behaviour of FileSystemStorage. In addition to the new kwarg, this adds a test for the existing no-overwriting behaviour, and a test for the newly enabled overwriting behaviour.

I'm not sure about what I should enter here for "Version" (could go in anywhere, really) and "Has patch" (I don't have a patch, but I do have a PR-able branch, linked above. I am assuming that I should wait for review here before actually submitting a PR, though)

HOW?

Just add an allow_overwrite kwarg to FileSystemStorage._save that a subclasser can optionally take advantage of.

WHY?

You can't do this in a good way today.

A piece of advice I've seen in several places (see Refs) for creating an overwriting storage backend is to create a subclass of FileSystemStorage where the get_available_name deletes an existing file of the same name. This, in my opinion, is a sneaky use of this function, based on the name. I guess it works, but that function is not called get_available_name_and_maybe_delete_some_stuff_also

A less sneaky (but more race-conditiony) implementation I have seen is to have _save delete the existing file before calling FileSystemStorage._save.

One of these is deceptive and one loops forever if you're unlucky.

Refs:

Various incarnations of this "ovewriting" implementation:

Change History (3)

comment:1 Changed 7 months ago by Jon Prindiville

Description: modified (diff)

comment:2 Changed 7 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

It looks reasonable.

comment:3 Changed 7 months ago by Jon Prindiville

Has patch: set
Owner: changed from nobody to Jon Prindiville
Status: newassigned

PR

I'm not too sure about adding to the release notes or AUTHORS file, so I'm leaving that alone for the time being.

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