[Django] #29890: storage: catch the right exception on concurrent os.mkdirs
Django
------------------------------------------------+------------------------
Reporter: mar77i | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29890>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: mar77i | Owner: Vishvajit
| Pathak
Type: Bug | Status: assigned
Component: File | Version: 2.1
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Vishvajit Pathak):
* cc: Vishvajit Pathak (added)
* owner: nobody => Vishvajit Pathak
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29890#comment:1>
Django
os.mkdirs()
-------------------------------------+-------------------------------------
Reporter: mar77i | Owner: Vishvajit
| Pathak
Type: Bug | Status: assigned
Component: File | Version: 2.1
uploads/storage |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
It looks like my mistake in 632c4ffd9cb1da273303bcd8005fff216506c795. We
can add a test by mocking `os.makedirs` to raise `FileExistsError`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29890#comment:2>
Django
os.mkdirs()
-------------------------------------+-------------------------------------
Reporter: mar77i | Owner: Vishvajit
| Pathak
Type: Bug | Status: assigned
Component: File | Version: 2.1
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Srinivas Reddy Thatiparthy):
Vishvajit ,
Are your working on a fix?
--
Ticket URL: <https://code.djangoproject.com/ticket/29890#comment:3>
Django
os.mkdirs()
-------------------------------------+-------------------------------------
Reporter: mar77i | Owner: Vishvajit
| Pathak
Type: Bug | Status: assigned
Component: File | Version: 2.1
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* has_patch: 0 => 1
Comment:
I created a [https://github.com/django/django/pull/10593 PR] so we can get
this fixed in tomorrow's bug fix release.
--
Ticket URL: <https://code.djangoproject.com/ticket/29890#comment:4>
Django
os.mkdirs()
-------------------------------------+-------------------------------------
Reporter: mar77i | Owner: Vishvajit
| Pathak
Component: File | Version: 2.1
uploads/storage |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"98ef3829e96ebc73d4d446f92465e671ff520d2b" 98ef3829]:
{{{
#!CommitTicketReference repository=""
revision="98ef3829e96ebc73d4d446f92465e671ff520d2b"
Fixed #29890 -- Fixed FileSystemStorage crash if concurrent saves try to
create the same directory.
Regression in 632c4ffd9cb1da273303bcd8005fff216506c795.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29890#comment:5>
Django
os.mkdirs()
-------------------------------------+-------------------------------------
Reporter: mar77i | Owner: Vishvajit
| Pathak
Type: Bug | Status: closed
Component: File | Version: 2.1
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"cd7d6c8af7e9ecfd033c584ba50c782d6b82817c" cd7d6c8a]:
{{{
#!CommitTicketReference repository=""
revision="cd7d6c8af7e9ecfd033c584ba50c782d6b82817c"
[2.1.x] Fixed #29890 -- Fixed FileSystemStorage crash if concurrent saves
try to create the same directory.
Regression in 632c4ffd9cb1da273303bcd8005fff216506c795.
Backport of 98ef3829e96ebc73d4d446f92465e671ff520d2b from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29890#comment:6>