m1racoli edited a comment on pull request #22475:
URL: https://github.com/apache/airflow/pull/22475#issuecomment-1077425782
> @potiuk but isn't using `tempfile.TemporaryDirectory` without entering the
context manager simply a mistake? I don't think one should rely on the
finalizer behavior in order to close such resources.
(my opinionated view)
I would disagree. I see the context manager more as an extra and getting
deleted upon "destruction of the temporary directory object" is a core feature
of `tempfile.TemporaryDirectory`(let's ignore `kill -9` and other scenarios
where a context manager doesn't help either).
Furthermore using a context manager is not alway practical. In particular,
we use it in a factory for task specific tmp dirs, which can be called multiple
times within a task in different places:
```python
def tmpdir() -> TemporaryDirectory:
"""
Provides a task instance specific instance of a temporary directory.
On first invocation a new directory will be created and the holding
object will be
stored in the task instance's context. Successive calls return the
already allocated object.
"""
ctx = python.get_current_context()
d = ctx.get("tmpdir")
if d is None:
d = TemporaryDirectory(prefix=f"{ctx['task_instance_key_str']}.")
logging.info(f"Temporary directory {d.name} created")
ctx["tmpdir"] = d
return d
```
It is of course debatable if this is a good thing to do (and the
implementation might only be half-baked).
In regards to the general PR, the TemporaryDirectory issue can be considered
as a symptom of a more fundamental problem which extends beyond just this case
(as mentioned by @potiuk in regards to open-telemetry which I highly anticipate
:) ).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
