bpo-29982: Add "ignore_cleanup_errors" param to tempfile.TemporaryDirectory() by CAM-Gerlach · Pull Request #24793 · python/cpython (original) (raw)
Resolves bpo-29982 by adding an ignore_cleanup_errors parameter to tempfile.TemporaryDirectory(), which allows for a best-effort cleanup without stopping on errors, such as PermissionErrors on Windows due to files being open, transient access failures or other runtime exceptions. This behavior matches that when passing ignore_errors=True to shutil.rmtree, and allows using the nicer higher-level interface, context-manager support and more robust removal implementation of tempfile.TemporaryDirectory() to clean up as much of the temporary directory as possible, without stopping and raising an exception mid-cleanup, which can otherwise potentially occur at a non-deterministic time when the directory is garbage-collected, or during interpreter shutdown.
Additionally, as the other part of the issue described in bpo-29982 (and as required for the tests and many real-world use cases), ensures that cleanup() runs correctly on subsequent attempts if the directory was not completely cleaned up after the first call, instead of silently (and surprisingly to the user) failing to do anything at all.
Example use cases for this include regebro/pyroma#28 (which prompted this proposal), the original scenario described in bpo-29982 , and one of the two use cases described by @asottile in bpo-25024 Since I assume everything will be squashed to one commit anyway, I've split this up into several to make it easier to review and modify.
Key implementation questions to draw reviewer attention to:
- Do we want to call the parameter
ignorecleanuperrorsor something else? Its on the long side, but the shorter alternatives I considered (e.g.ignore_errors) were not as clear and descriptive.ignore_errorsis whatshutil.rmtreeuses but given its in theTemporaryDirectoryconstructor, it could otherwise mislead users into thinking that errors creating the temporary directory would be ignored, when in fact this is only the case for cleanup. - Should we add an
ignoreerrorsoptional parameter tocleanup()as well? This would be a simple but useful addition so calling code can explicitly specify at manual cleanup time whether they want an error to be returned, rather than being locked in to one or the other after object creation (and would allow for more robust unit tests), but I didn't add it yet to avoid it getting out of scope. - Should the new tests be modified (somehow?) to guarantee initial removal will fail on all platforms? Given the multiple mitigations against removal failing for various reasons, the simplest way to reliably cause removal to fail on real files was just leaving it open, but that only actually causes an error on Windows, AFAIK. This means that
ignore_errors=Trueis tested with errors on Windows, and without them on Linux and macOS. Is there a straightforward way to ensure an error will occur on *nix, in a way that the existing cleanup code cannot handle? Maybe OS-specific locks? - Does it need a Whats New entry? I didn't add one since it shouldn't have backward compatibility implications and its just adding a single parameter (along with a corresponding fix to the behavior of
cleanup), but I do see some other entries related to adding new parameters to callables, so I'm not sure.
Thanks!