Josh Rosenberg <shadowranger+pyt...@gmail.com> added the comment:

Allowing delete_on_error=False is kind of defeating the purpose here; the whole 
point of the with statement is guaranteed, consistent cleanup in both normal 
and error cases. If you knew enough to know you needed to pass delete_on_error, 
you'd also know enough to know you should be handling errors properly in the 
first place, e.g. by changing your mount_sshfs manager to:

    @contextmanager
    def mount_sshfs(localdir, remote):
        subprocess.run(f"sshfs {remote} {localdir}")
        try:
            yield
        finally:
            subprocess.run(f"fusermount -u {localdir}", check=True)

so it actually performed the guaranteed cleanup you expected from it.

I don't see anything wrong with adding a delete=True argument to 
TemporaryDirectory, though I'm not seeing it as being as useful as it is with 
TemporaryFile (since the "rewrite file to tempfile, atomic replace old file 
with new file" pattern for updating a file safely doesn't transfer directly to 
directories, where atomic renames aren't an option).

It just seems like your fundamental problem is code that doesn't properly 
handle failure, and I don't think the solution is to make TemporaryDirectory 
handle failure badly as well.

----------
nosy: +josh.r

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue36422>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to