[Django] #32821: Use a with statement when using os.scandir()
Django
------------------------------------------------+------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: dev
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 |
------------------------------------------------+------------------------
I noticed that Django doesn't use a `with` statement when it uses
`os.scandir()`, which is what the
[https://docs.python.org/3/library/os.html#os.scandir.close Python docs
advise as of 3.6]. Here is
[https://github.com/django/django/blob/ecf8af79355c8daa67722bd0de946b351f7f613d/django/core/files/storage.py#L324-L328
one example in the code]. There appear to be 7 uses of `os.scandir()` in
all, with 5 in test code.
Usage with the `with` statement looks like this:
{{{#!python
with os.scandir(path) as entries:
for entry in entries:
...
}}}
Not using a `with` statement or closing the iterator can cause a
`ResourceWarning`, e.g. when an exception is raised.
--
Ticket URL: <https://code.djangoproject.com/ticket/32821>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Uncategorized | Version: dev
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
Description changed by Chris Jerdonek:
Old description:
> I noticed that Django doesn't use a `with` statement when it uses
> `os.scandir()`, which is what the
> [https://docs.python.org/3/library/os.html#os.scandir.close Python docs
> advise as of 3.6]. Here is
> [https://github.com/django/django/blob/ecf8af79355c8daa67722bd0de946b351f7f613d/django/core/files/storage.py#L324-L328
> one example in the code]. There appear to be 7 uses of `os.scandir()` in
> all, with 5 in test code.
>
> Usage with the `with` statement looks like this:
>
> {{{#!python
> with os.scandir(path) as entries:
> for entry in entries:
> ...
> }}}
>
> Not using a `with` statement or closing the iterator can cause a
> `ResourceWarning`, e.g. when an exception is raised.
New description:
I noticed that Django doesn't use a `with` statement (or call `close()`)
when it uses `os.scandir()`, which is what the
[https://docs.python.org/3/library/os.html#os.scandir.close Python docs
advise as of 3.6]. Here is
[https://github.com/django/django/blob/ecf8af79355c8daa67722bd0de946b351f7f613d/django/core/files/storage.py#L324-L328
one example in the code]. There appear to be 7 uses of `os.scandir()` in
all, with 5 in test code.
Usage with the `with` statement looks like this:
{{{#!python
with os.scandir(path) as entries:
for entry in entries:
...
}}}
Not using a `with` statement or closing the iterator can cause a
`ResourceWarning`, e.g. when an exception is raised.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:1>
Django
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Chris Jerdonek):
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/14497
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:2>
Django
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Chris Jerdonek:
Old description:
> I noticed that Django doesn't use a `with` statement (or call `close()`)
> when it uses `os.scandir()`, which is what the
> [https://docs.python.org/3/library/os.html#os.scandir.close Python docs
> advise as of 3.6]. Here is
> [https://github.com/django/django/blob/ecf8af79355c8daa67722bd0de946b351f7f613d/django/core/files/storage.py#L324-L328
> one example in the code]. There appear to be 7 uses of `os.scandir()` in
> all, with 5 in test code.
>
> Usage with the `with` statement looks like this:
>
> {{{#!python
> with os.scandir(path) as entries:
> for entry in entries:
> ...
> }}}
>
> Not using a `with` statement or closing the iterator can cause a
> `ResourceWarning`, e.g. when an exception is raised.
New description:
I noticed that Django doesn't use a `with` statement (or call `close()`)
when it uses `os.scandir()`, which is what the
[https://docs.python.org/3/library/os.html#os.scandir.close Python docs
advise as of 3.6]. Here is
[https://github.com/django/django/blob/ecf8af79355c8daa67722bd0de946b351f7f613d/django/core/files/storage.py#L324-L328
one example in the code]. There appear to be 7 uses of `os.scandir()` in
all, with 5 in test code.
Usage with the `with` statement looks like this:
{{{#!python
with os.scandir(path) as entries:
for entry in entries:
...
}}}
Not using a `with` statement or closing the iterator can cause a
`ResourceWarning`, e.g. when an exception is raised. Indeed, this is how I
first noticed this issue.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:3>
Django
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):
* owner: nobody => Chris Jerdonek
* status: new => assigned
* component: Uncategorized => Testing framework
* easy: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:4>
Django
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
Two of the occurrences are in non-test code
(`django/core/files/storage.py` and `django/forms/fields.py`), so "Testing
framework" doesn't seem quite right to me. It gives the impression non-
test code isn't affected.
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:5>
Django
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* component: Testing framework => Core (Other)
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:6>
Django
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
Thanks, Claude.
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:7>
Django
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Component: Core (Other) | Version: dev
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"7272e1963ffdf39c1d4fe225d5425a45dd095d11" 7272e196]:
{{{
#!CommitTicketReference repository=""
revision="7272e1963ffdf39c1d4fe225d5425a45dd095d11"
Fixed #32821 -- Updated os.scandir() uses to use a context manager.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32821#comment:8>