Opened 4 years ago
Closed 4 years ago
#32821 closed Cleanup/optimization (fixed)
Use a with statement when using os.scandir()
| Reported by: | Chris Jerdonek | Owned by: | Chris Jerdonek |
|---|---|---|---|
| Component: | Core (Other) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
I noticed that Django doesn't use a with statement (or call close()) when it uses os.scandir(), which is what the Python docs advise as of 3.6. Here is 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:
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.
Change History (8)
comment:1 by , 4 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 4 years ago
| Has patch: | set |
|---|
comment:3 by , 4 years ago
| Description: | modified (diff) |
|---|
comment:4 by , 4 years ago
| Component: | Uncategorized → Testing framework |
|---|---|
| Easy pickings: | set |
| Owner: | changed from to |
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
comment:5 by , 4 years ago
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.
comment:6 by , 4 years ago
| Component: | Testing framework → Core (Other) |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
PR: https://github.com/django/django/pull/14497