[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Maksim Beliaev]

Maksim Beliaev  added the comment:

just want to correct code that Nils post in the last comment to really take 
args and kwargs

~~~
def mkdtemp_persistent(*args, persistent=True, **kwargs):
if persistent:
@contextmanager
def normal_mkdtemp():
yield tempfile.mkdtemp(*args, **kwargs)
return normal_mkdtemp()
else:
return tempfile.TemporaryDirectory(*args, **kwargs)

with mkdtemp_persistent(persistent=False) as dir:
...
~~~

--
nosy: +beliaev-maksim

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Nils Kattenbeck]

Nils Kattenbeck  added the comment:

While the proposal linked by C.A.M. solves one of the use cases but it does not 
address the others.

One use cases which is rather common for me it is e.g. to have scripts/programs 
which allow configuring whether temporary directories should get deleted or 
stay persistent after program exit.
Currently this always requires hand rolled wrappers like the following:

def mkdtemp_persistent(*args, persistent=True, **kwargs):
if persistent:
@contextmanager
def normal_mkdtemp():
yield tempfile.mkdtemp()
return normal_mkdtemp(*args, **kwargs)
else:
return tempfile.TemporaryDirectory(*args, **kwargs)

with mkdtemp_persistent(persistent=False) as dir:
...

Which gets the job done but is not as user friendly as other parts of the 
stdlib.

--
nosy: +Nils Kattenbeck

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[C.A.M. Gerlach]

Change by C.A.M. Gerlach :


--
pull_requests: +23561
pull_request: https://github.com/python/cpython/pull/24793

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[C.A.M. Gerlach]

C.A.M. Gerlach  added the comment:

To note, the proposal on [BPO-29982](https://bugs.python.org/issue29982) should 
hopefully address one of the two use-cases described by Anthony Sotittle, 
`ignore_errors=True`, in a cleaner fashion that takes advantage of the existing 
higher-level functionality rather than duplicating `mkdtemp`. Opinions and 
feedback welcome over there.

--
nosy: +CAM-Gerlach

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Ashwin Ramaswami]

Ashwin Ramaswami  added the comment:

I agree -- as a user, it wasn't clear to me from looking at the documentation 
that mkdtemp was the right way to go to not delete directories. I had expected 
that NamedTemporaryDirectory would also support delete=False, just like 
NamedTemporaryFile.


Given that we say in the documentation that "mkstemp() and mkdtemp() are 
lower-level functions which require manual cleanup", it seems to make sense to 
allow Python users who don't care / know about mkstemp / mkdtemp to just use 
the more user-friendly NamedTemporaryDirectory / NamedTemporaryFile classes. 
This would make the language more accessible.

Would we be fine with reopening this issue so someone can contribute a patch?

--
nosy: +epicfaace

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Richard]

Richard  added the comment:

Sorry for reviving a 9 months old issue, but IMO there was no good reason to 
reject this especially when a patch was provided. Even if the context manager 
can be replaced with 3 lines of code, I still don't consider that very 
user-friendly.

My use case would be passing `delete=False` temporarily while debugging my 
script, it would be much simpler than using a whole different hacky method when 
the end goal is to change it back to `delete=True` once it is completed anyway.

What issues exactly does the addition of a simple option cause? I don't think 
something as trivial as this causes a maintenance burden, and you can't call it 
feature creep either when TemporaryDirectory doesn't have *any* other optional 
keyword arguments.

--
nosy: +nyuszika7h

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Well, then I think there is nothing to do this.

Compatibility with context manager protocol is not a goal if it does nothing on 
exit. In any case you can easy to make it in three lines:

@contextmanager
def mymkdtemp():
yield mkdtemp()

NamedTemporaryFile is more complex than mkstemp(). It creates a file object and 
closes a file descriptor even if leave the file on the filesystem.

--
resolution:  -> rejected
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Anthony Sottile]

Anthony Sottile  added the comment:

you are right though, the effect is the same as just using mkdtemp

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Anthony Sottile]

Anthony Sottile  added the comment:

the differences are api compatibility with context manager protocol and 
equivalence with NamedTemporaryFile

NamedTemporaryFile(delete=False) is just `mkstemp` afaict and the api is there

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Could you please explain the difference between hypothetical 
TemporaryDirectory(delete=False) and simple mkdtemp()?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Anthony Sottile]

Anthony Sottile  added the comment:

Serhiy's comment provides workarounds but are still (imo) inferior to 
supporting this.

I would really like for this api to match that of NamedTemporaryFile which has 
the same `delete=...` option

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Mauro S. M. Rodrigues]

Mauro S. M. Rodrigues  added the comment:

So per Serhiy comment can I assume the patch is not necessary? If so I believe 
the issue should be closed as well.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Anthony Sottile]

Anthony Sottile  added the comment:

oh!  that's neat, yeah hadn't been following closely enough it seems, good to 
hear that the readonly thing is fixed!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

If you want the conditional cleanup, then TemporaryDirectory is obviously a 
wrong tool. mkdtemp looks more appropriate.

If you remove directory in process of working with temporary directory, would 
it help if you create a subdirectory in the temporary directory and work with 
it (including conditionally removing)?

As for the readonly files, was not this problem solved by PR 10320?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Anthony Sottile]

Anthony Sottile  added the comment:

one example is here: 
https://github.com/pre-commit/pre-commit/blob/bb6f1efe63c168d9393d520bd60e16c991a57059/pre_commit/store.py#L137-L139

where I would want cleanup in the exceptional case

another (related but different) closed-source use case involves creating a 
temporary directory that may be deleted and currently has to be guarded by:

shutil.rmtree(..., ignore_errors=True)

(making `TemporaryDirectory` not useful since it crashes if the directory goes 
missing)

there's additionally the problem of readonly files on windows, and readonly 
directories elsewhere being undeletable without a custom rmtree but I think 
that's a different issue entirely -- 
https://github.com/pre-commit/pre-commit/blob/bb6f1efe63c168d9393d520bd60e16c991a57059/pre_commit/util.py#L250-L267

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

What is your use case Anthony?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Antony Lee]

Change by Antony Lee :


--
nosy:  -Antony.Lee

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Mauro S. M. Rodrigues]

Mauro S. M. Rodrigues  added the comment:

Hi Anthony, 

Thanks for asking, yeah I'm interested in push a new version. I'll do it later 
today and I'll post a link to the pr here.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Anthony Sottile]

Anthony Sottile  added the comment:

I would certainly like to see this, it would eliminate my last few hand rolled 
temporary directory contexts

Mauro would you be interested in re-posting this patch as a PR to github? (or 
allowing someone else to carry your patch forward?)

--
nosy: +Anthony Sottile

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Mauro S. M. Rodrigues]

Mauro S. M. Rodrigues added the comment:

Hi everybody!
This is my second patch on the community, although the first one is not merged, 
so any feedback is appreciated. 

I've added tests to cover this new situation and docs to let people know about 
the possibility of keeping their temporary directories for debugging purposes. 

Regarding the code itself, I've also made a 'design decision' to remove the 
directory even when created with delete=False if cleanup method is called 
explicitly, so I would like to hear your thoughts on that matter specially if 
you don't agree with it.

--
keywords: +patch
nosy: +maurosr
Added file: 
http://bugs.python.org/file40694/TemporaryDirectory_delete_false.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I'm inclined to reject this idea too.

--
nosy: +georg.brandl, ncoghlan, pitrou, serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Antoine Pitrou]

Antoine Pitrou added the comment:

The idea looks ok to me, but you'd have to provide a patch.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[R. David Murray]

R. David Murray added the comment:

The two cases are not parallel, in that NamedTemporaryFile closes the file at 
the end of the context manager, whereas the *only* thing the TemporaryDirectory 
does is delete the directory on cleanup.

There is some value to the "parallel interface" argument, so I'm only -0 on 
this.

--
nosy: +r.david.murray
type:  -> enhancement

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25024] Allow passing "delete=False" to TemporaryDirectory

[Antony Lee]

New submission from Antony Lee:

I would like to suggest allowing passing "delete=False" to the 
TemporaryDirectory constructor, with the effect that the directory is not 
deleted when the TemporaryDirectory context manager exits, or when the 
TemporaryDirectory object is deleted.

I realize that this would effectively duplicate the functionality of mkdtemp, 
but this is similar to the redundancy between NamedTemporaryFile(delete=False) 
and mkstemp, and would make it easier to switch between the two behaviors 
(which is useful e.g. when debugging, where you may need to look at the 
temporary files "post-mortem").

--
components: Library (Lib)
messages: 250158
nosy: Antony.Lee
priority: normal
severity: normal
status: open
title: Allow passing "delete=False" to TemporaryDirectory
versions: Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com