[Django] #24664: TemporaryFileUploadHandler new_file() does not match its parent class
Django
-------------------------------+--------------------
Reporter: ddanier | Owner: nobody
Type: Uncategorized | Status: new
Component: Core (Other) | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------
TemporaryFileUploadHandler.new_file(self, file_name, *args, **kwargs)
vs.
FileUploadHandler.new_file(self, field_name, file_name, content_type,
content_length, charset=None, content_type_extra=None)
TemporaryFileUploadHandler.new_file will take "file_name" as its first
argument and pass this (still as first argument) to
FileUploadHandler.new_file. So the argument will end up
FileUploadHandler.field_name, which is wrong.
I think a valid fix would be to remove file_name from
TemporaryFileUploadHandler.new_file as the parameter is not used besides
for calling the parent class. It may have been used for passing it to
TemporaryUploadedFile, but this must be history. I'll attach a patch to
change this.
--
Ticket URL: <https://code.djangoproject.com/ticket/24664>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: ddanier | Owner: nobody
Type: Uncategorized | Status: new
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 1
UI/UX: 0 |
Changes (by ddanier):
* Attachment "TemporaryFileUploadHandler_new_file_fix_parent_call.patch"
added.
Django
Reporter: ddanier | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Ready for checkin
Comment:
Indeed, the parameter name is misleading, but this shouldn't lead to
problems as the parameter is not used (this issue is there from 2008 and
[d725cc9734272f867d41f7236235c28b3931a1b2]).
The patch looks good.
--
Ticket URL: <https://code.djangoproject.com/ticket/24664#comment:1>
Django
Reporter: ddanier | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by ddanier):
> [...] but this shouldn't lead to problems as the parameter is not used
It does lead to problems if you call TemporaryFileUploadHandler.new_file()
with named parameters (kwargs). In this case you will pass mutiple values
for the same argument when calling super(). Hit me here:
https://github.com/ddanier/django-git-
lfs/blob/master/django_git_lfs/views.py#L130
(Fixed this by only passing normal args)
--
Ticket URL: <https://code.djangoproject.com/ticket/24664#comment:2>
Django
Reporter: ddanier | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Core (Other) | Version: 1.8
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"9346e45f84e893363e4c0d9b5b4bcb1a62434c08" 9346e45f]:
{{{
#!CommitTicketReference repository=""
revision="9346e45f84e893363e4c0d9b5b4bcb1a62434c08"
Fixed #24664 -- Removed misleading arg name from
TemporaryFileUploadHandler.new_file
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24664#comment:3>