#29890 closed Bug (fixed)
FileSystemStorage._save() doesn't catch FileExistsError on concurrent os.mkdirs()
Reported by: | mar77i | Owned by: | Vishvajit Pathak |
---|---|---|---|
Component: | File uploads/storage | Version: | 2.1 |
Severity: | Normal | Keywords: | |
Cc: | Vishvajit Pathak | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
My django tests (ran in --parallel
) raised an exception which Django appears to attempt to be dealing with already:
Traceback (most recent call last): [...] File "[app dir]/models.py", line 56, in create_user return self._create_user(email, password, **extra_fields) File "[app dir]/models.py", line 50, in _create_user user.save(using=self._db) File "[app dir]/models.py", line 157, in save save=False File "[project dir]venv/lib/python3.5/site-packages/django/db/models/fields/files.py", line 87, in save self.name = self.storage.save(name, content, max_length=self.field.max_length) File "[project dir]venv/lib/python3.5/site-packages/django/core/files/storage.py", line 49, in save return self._save(name, content) File "[project dir]venv/lib/python3.5/site-packages/django/core/files/storage.py", line 236, in _save os.makedirs(directory) File "/usr/lib/python3.5/os.py", line 241, in makedirs mkdir(name, mode) FileExistsError: [Errno 17] File exists: '[storage dir]'
Inspecting storage.py showes that os.makedirs() is guarded with a try...except block concerning FileNotFoundError, but apparently, FileExistsError appears to be what that exception handler was supposed to be catching.
Change History (6)
comment:1 by , 6 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:2 by , 6 years ago
Summary: | storage: catch the right exception on concurrent os.mkdirs → FileSystemStorage._save() doesn't catch FileExistsError on concurrent os.mkdirs() |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 6 years ago
Has patch: | set |
---|
I created a PR so we can get this fixed in tomorrow's bug fix release.
Note:
See TracTickets
for help on using tickets.
It looks like my mistake in 632c4ffd9cb1da273303bcd8005fff216506c795. We can add a test by mocking
os.makedirs
to raiseFileExistsError
.