[Django] #24105: When upload_to is callable, storage.get_valid_name() does not get called
Django
-------------------------------------+-------------------------------------
Reporter: jbowen7 | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) | Keywords: upload_to,
Severity: Normal | get_valid_name
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
== Module: db.models.fields.files.py ==
Present in recent commits of versions 1.8 and 1.7
Commit: c1493879d98328040b36ad93cd104c0b3b546772
Commit: bbcbacf0ad4fc9aa89d13c35f4b084185b284a49
Summary summed it up. But maybe the Storage class should be calling
get_valid_name like the docs suggest. -see (_save)
[https://docs.djangoproject.com/en/1.7/howto/custom-file-storage/]
{{{
"_save(name, content)¶
Called by Storage.save(). The name will already have gone through
get_valid_name() and get_available_name(), and the content will be a File
object itself."
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24105>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: jbowen7 | Owner: nobody
Cleanup/optimization |
Component: File | Version: 1.7
uploads/storage |
Severity: Normal | Resolution:
Keywords: upload_to, | Triage Stage: Accepted
get_valid_name |
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: => 0
* component: Database layer (models, ORM) => File uploads/storage
* needs_tests: => 1
* needs_docs: => 1
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
This might be a reasonable change, but we need to think if it might break
backwards compatibility in any way. It will also need tests and docs.
--
Ticket URL: <https://code.djangoproject.com/ticket/24105#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: jbowen7 | Owner: nobody
Cleanup/optimization |
Component: File | Version: 1.7
uploads/storage |
Severity: Normal | Resolution:
Keywords: upload_to, | Triage Stage: Accepted
get_valid_name |
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
The patch raises a `SyntaxError` at the following line:
{{{#!diff
+ directory_name, filename =
os.path.split(self.upload_to(instance, filename)
}}}
The `else` clause is not needed:
{{{#!python
if callable(self.upload_to):
# do stuff
return os.path.join(self.get_directory_name(),
self.get_filename(filename))
}}}
Indentation should be 4 spaces.
--
Ticket URL: <https://code.djangoproject.com/ticket/24105#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: jbowen7 | Owner: nobody
Cleanup/optimization |
Component: File | Version: master
uploads/storage |
Severity: Normal | Resolution:
Keywords: upload_to, | Triage Stage: Accepted
get_valid_name |
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 1.7 => master
Comment:
With this change, the documentation of get_valid_name should also be
updated to clarify that the passed name might come from `upload_to` also.
[https://docs.djangoproject.com/en/1.7/howto/custom-file-storage/]
{{{
get_valid_name(name)¶
Returns a filename suitable for use with the underlying storage system.
The name argument passed to this method is the original filename sent to
the server, after having any path information removed.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24105#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: File | Version: master
uploads/storage |
Severity: Normal | Resolution:
Keywords: upload_to, | Triage Stage: Accepted
get_valid_name |
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => abhaga
--
Ticket URL: <https://code.djangoproject.com/ticket/24105#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: File | Version: master
uploads/storage |
Severity: Normal | Resolution:
Keywords: upload_to, | Triage Stage: Accepted
get_valid_name |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
I have created a pull request at
[https://github.com/django/django/pull/4603] with documentation & test
cases based on the original patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/24105#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: File | Version: master
uploads/storage |
Severity: Normal | Resolution:
Keywords: upload_to, | Triage Stage: Accepted
get_valid_name |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* cc: abhaga (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24105#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: File | Version: master
uploads/storage |
Keywords: upload_to, | Triage Stage: Accepted
get_valid_name |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"9de9c240178bd9f61f55dad07e76ea458f0291f5" 9de9c240]:
{{{
#!CommitTicketReference repository=""
revision="9de9c240178bd9f61f55dad07e76ea458f0291f5"
Fixed #24105 -- Called Storage.get_valid_name() when upload_to is callable
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24105#comment:7>