potiuk edited a comment on pull request #22475: URL: https://github.com/apache/airflow/pull/22475#issuecomment-1077288905
> @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. Technically speaking it **should** work: https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryDirectory > This function securely creates a temporary directory using the same rules as [mkdtemp()](https://docs.python.org/3/library/tempfile.html#tempfile.mkdtemp). The resulting object can be used as a context manager (see [Examples](https://docs.python.org/3/library/tempfile.html#tempfile-examples)). On completion of the context or destruction of the temporary directory object, the newly created temporary directory and all its contents are removed from the filesystem. And I read a lot about it - while traditionally relying on finalizers was not really good idea in Python 2, and early Python 3 versions, it started to work much more reliable ~ Python 3.5, and unless there is a very good reason (like kill -9) it **should** work in all "normal" cases. Of course blindly relying that finalizer will **always** be called is not a good idea, expecting that thy will be called in all "normal" situation is quite reaasonable. Just the fact that our user expected it and it did not work is a statemet on it's own. And there might be many other cases like that. For example I know for a fact that open-telemetry also registers some finalizers (and we are going to add it soon - we have an AIP for it cooking) and while deleting TemporaryDirectory is not a big issue, not calling open-telemetry finalizers is. If we prevent finalizers from being called, then we also prevent open-telemetry from sending the cachesd information about the last failure which caused an application crash. This is precisely this kind of problem that we want to avoid (and when it comes to open telemetry - this is precisely the case that open-telemetry is there to help our users with). So yeah. I think preventing finalizer from being called when it **should** be called and there are no "system evnts" that prevent it, is actually a BIG deal and we **must** fix it. > That said, this is much better (exiting outside of the try/finally block) – but why are we even having a try/finally-block at all. > The exception clause catches everything. Not really. Finally clause is also executed when there is no exception at all. -- 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]
