[Django] #28144: Add allow_overwrite kwarg to FileSystemStorage._save
Django
-------------------------------------+-------------------------------------
Reporter: Jon | Owner: nobody
Prindiville |
Type: New | Status: new
feature |
Component: File | Version: 1.11
uploads/storage | Keywords: file storage
Severity: Normal | overwrite
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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
:filesystemstorage-allow-overwrite) 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 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 Refs]:
Various incarnations of this "ovewriting" implementation:
- django-storages, marked as deprecated (https://github.com/jschneier
/django-storages/blob/1.5.2/storages/backends/overwrite.py)
- djangosnippets (https://djangosnippets.org/snippets/976/)
- someone's gist (https://gist.github.com/fabiomontefuscolo/1584462)
- django-overwrite-storage (https://github.com/ckot/django-overwrite-
storage/blob/master/overwrite_storage/storage.py)
--
Ticket URL: <https://code.djangoproject.com/ticket/28144>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: File | Version: 1.11
uploads/storage |
Keywords: file storage | Triage Stage:
overwrite | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
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)
=== HOW?
=== WHY?
=== [=#refs Refs]:
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28144#comment:1>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
uploads/storage |
Keywords: file storage | Triage Stage: Accepted
overwrite |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
It looks reasonable.
--
Ticket URL: <https://code.djangoproject.com/ticket/28144#comment:2>
Django
-------------------------------------+-------------------------------------
| Prindiville
Type: New feature | Status: assigned
Component: File | Version: 1.11
uploads/storage |
Keywords: file storage | Triage Stage: Accepted
overwrite |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Jon Prindiville
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/8476 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28144#comment:3>
Django
-------------------------------------+-------------------------------------
| Prindiville
Type: New feature | Status: assigned
uploads/storage |
Keywords: file storage | Triage Stage: Accepted
overwrite |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28144#comment:4>
Django
-------------------------------------+-------------------------------------
| Prindiville
Type: New feature | Status: assigned
uploads/storage |
Keywords: file storage | Triage Stage: Accepted
overwrite |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Roland van Laar):
Replying to [comment:4 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/28144#comment:5>
Django
-------------------------------------+-------------------------------------
| Prindiville
Component: File | Version: 1.11
uploads/storage |
Keywords: file storage | Triage Stage: Accepted
overwrite |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b4cba4ed625ce7c88675616b3bbb237c28a926d1" b4cba4e]:
{{{
#!CommitTicketReference repository=""
revision="b4cba4ed625ce7c88675616b3bbb237c28a926d1"
Fixed #28144 -- Added FileSystemStorage.OS_OPEN_FLAGS to allow
customization.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28144#comment:6>