Opened 8 months ago
Closed 8 months ago
#35323 closed Cleanup/optimization (fixed)
Fix FileExistsError in django/core/files/move.py.
Reported by: | bcail | Owned by: | bcail |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | bcail | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In file_move_safe(), if the destination exists and allow_overwrite is False, FileExistsError is raised - but it's immediately swallowed because it's inside a try block. The code goes on to try the manual copy process, and a new FileExistsError is raised in that code.
Change History (7)
comment:1 by , 8 months ago
Component: | Uncategorized → File uploads/storage |
---|---|
Has patch: | set |
comment:2 by , 8 months ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → Cleanup/optimization |
Makes sense, good catch!
I wonder if we could add a test that ensures that the first FileExistsError
is not swallowed. Do you want to give that a try?
comment:3 by , 8 months ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:4 by , 8 months ago
Needs tests: | unset |
---|
Thanks, Natalia. I updated the PR with a test for that specific FileExistsError
.
comment:5 by , 8 months ago
Patch needs improvement: | unset |
---|
comment:6 by , 8 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Triage Stage: | Accepted → Ready for checkin |
I opened a PR.