-
-
Notifications
You must be signed in to change notification settings - Fork 34.6k
bpo-14243: Optionally delete NamedTemporaryFile on content manager exit but not on file close #22431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-14243: Optionally delete NamedTemporaryFile on content manager exit but not on file close #22431
Changes from 23 commits
144db7f
21f7df0
2d7927b
4fcddaf
76f28f3
6be9be6
faf1a34
589d0b8
e22a3db
3be2f47
39e8b50
4d8f152
9ec55f5
d6f382e
f9dc427
e29b7d2
d003a39
e2fc6db
3ac4e20
653cf27
d5d07f4
0b32b28
5773e71
d98f270
9598e6b
5a7e177
7e7ad85
4d5677e
cf5b1b5
7fd339a
da71a96
5e22c9c
b1f5152
07076e8
a0a8ae3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -421,10 +421,11 @@ class _TemporaryFileCloser: | |
| file = None # Set here since __del__ checks it | ||
| close_called = False | ||
|
|
||
| def __init__(self, file, name, delete=True): | ||
| def __init__(self, file, name, delete=True, delete_on_close=True): | ||
| self.file = file | ||
| self.name = name | ||
| self.delete = delete | ||
| self.delete_on_close = delete_on_close | ||
|
|
||
| # NT provides delete-on-close as a primitive, so we don't need | ||
| # the wrapper to do anything special. We still use it so that | ||
|
|
@@ -442,7 +443,7 @@ def close(self, unlink=_os.unlink): | |
| try: | ||
| self.file.close() | ||
| finally: | ||
| if self.delete: | ||
| if self.delete and self.delete_on_close: | ||
| unlink(self.name) | ||
|
|
||
| # Need to ensure the file is deleted on __del__ | ||
|
|
@@ -464,11 +465,13 @@ class _TemporaryFileWrapper: | |
| remove the file when it is no longer needed. | ||
| """ | ||
|
|
||
| def __init__(self, file, name, delete=True): | ||
| def __init__(self, file, name, delete=True, delete_on_close=True): | ||
| self.file = file | ||
| self.name = name | ||
| self.delete = delete | ||
| self._closer = _TemporaryFileCloser(file, name, delete) | ||
| self.delete_on_close = delete_on_close | ||
| self._closer = _TemporaryFileCloser(file, name, delete, | ||
| delete_on_close) | ||
|
|
||
| def __getattr__(self, name): | ||
| # Attribute lookups are delegated to the underlying file | ||
|
|
@@ -500,6 +503,15 @@ def __enter__(self): | |
| def __exit__(self, exc, value, tb): | ||
| result = self.file.__exit__(exc, value, tb) | ||
| self.close() | ||
| # if file is to be deleted, bot not on closure, deleting it now | ||
|
Ev2geny marked this conversation as resolved.
Outdated
|
||
| if self.delete and not self.delete_on_close: | ||
| try: | ||
| _os.unlink(self.name) | ||
| # It shall be Ok to ignore FileNotFoundError, because user may | ||
| # have deleted the file already before content manager came to it | ||
|
Ev2geny marked this conversation as resolved.
Outdated
|
||
| except FileNotFoundError: | ||
| pass | ||
|
|
||
|
Comment on lines
+506
to
+514
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A case hasn't been covered. If
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eryksun , I am just trying to figure out, whether this is the issue introduced by my changes or this is a separate issue, which existed before?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a new case to cover. Before there was just
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I think I understand now. Also the new test case needs to be created to cover this, I think. Correct?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now implemented in the new PR 97015 |
||
| return result | ||
|
|
||
| def close(self): | ||
|
|
@@ -521,17 +533,23 @@ def __iter__(self): | |
|
|
||
| def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, | ||
| newline=None, suffix=None, prefix=None, | ||
| dir=None, delete=True, *, errors=None): | ||
| dir=None, delete=True, *, errors=None, | ||
| delete_on_close=True): | ||
| """Create and return a temporary file. | ||
| Arguments: | ||
| 'prefix', 'suffix', 'dir' -- as for mkstemp. | ||
| 'mode' -- the mode argument to io.open (default "w+b"). | ||
| 'buffering' -- the buffer size argument to io.open (default -1). | ||
| 'encoding' -- the encoding argument to io.open (default None) | ||
| 'newline' -- the newline argument to io.open (default None) | ||
| 'delete' -- whether the file is deleted on close (default True). | ||
| 'delete' -- whether the file is deleted at the end. When exactly file gets | ||
| deleted (either on close or on context manager exit) is determined by | ||
| parameter delete_on_close. (default True). | ||
|
Ev2geny marked this conversation as resolved.
Outdated
|
||
| 'errors' -- the errors argument to io.open (default None) | ||
| The file is created as mkstemp() would do it. | ||
| 'delete_on_close' -- if 'delete', then determines whether file gets deleted | ||
| on close. Otherwise it gets deleted on context manager exit | ||
| (default True) | ||
|
Ev2geny marked this conversation as resolved.
Outdated
|
||
|
|
||
| Returns an object with a file-like interface; the name of the file | ||
| is accessible as its 'name' attribute. The file will be automatically | ||
|
|
@@ -548,7 +566,7 @@ def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, | |
|
|
||
| # Setting O_TEMPORARY in the flags causes the OS to delete | ||
| # the file when it is closed. This is only supported by Windows. | ||
| if _os.name == 'nt' and delete: | ||
| if _os.name == 'nt' and delete and delete_on_close: | ||
| flags |= _os.O_TEMPORARY | ||
|
|
||
| if "b" not in mode: | ||
|
|
@@ -559,7 +577,7 @@ def NamedTemporaryFile(mode='w+b', buffering=-1, encoding=None, | |
| file = _io.open(fd, mode, buffering=buffering, | ||
| newline=newline, encoding=encoding, errors=errors) | ||
|
|
||
| return _TemporaryFileWrapper(file, name, delete) | ||
| return _TemporaryFileWrapper(file, name, delete, delete_on_close) | ||
| except BaseException: | ||
| _os.unlink(name) | ||
| _os.close(fd) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| The :class:`tempfile.NamedTemporaryFile` has a new optional parameter *delete_on_close* |
Uh oh!
There was an error while loading. Please reload this page.