Opened 12 months ago

Last modified 4 weeks 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: yes
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 (5)

comment:1 Changed 12 months ago by Jon Prindiville

Description: modified (diff)

comment:2 Changed 12 months ago by Tim Graham

Triage Stage: UnreviewedAccepted

It looks reasonable.

comment:3 Changed 12 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.

comment:4 Changed 5 months ago by Tim Martin

Patch needs improvement: set

comment:5 in reply to:  4 Changed 4 weeks ago by Roland van Laar

Replying to Tim Martin:

I'm open to picking up this issue and working on this patch to get it accepted.
You set the patch to 'Needs improvement'.

Can you elaborate on what kind of improvements are needed?

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