wjddn279 commented on code in PR #62103:
URL: https://github.com/apache/airflow/pull/62103#discussion_r2828337316


##########
airflow-core/src/airflow/jobs/triggerer_job_runner.py:
##########
@@ -319,6 +318,17 @@ def __call__(self, processors: 
Iterable[structlog.typing.Processor]) -> WrappedL
         self.bound_logger = logger
         return logger
 
+    def __del__(self):
+        # Explicitly close the file descriptor when the logger is garbage 
collected.
+        if raw_logger := getattr(self.bound_logger, "_logger", None):
+            file_handle = getattr(raw_logger, "_file", None)
+        else:
+            return
+
+        if file_handle and not file_handle.closed:
+            file_handle.flush()
+            file_handle.close()

Review Comment:
   > In short something like this, where we store the handler and clear it to 
avoid diversion from that approach?
   
   I agree of this and it looks better! Thanks!
   
   I've thought about that situation and approach as well, but if we separate 
it with such an explicit method `close`, that method  must always be called 
after `upload_to_remote` is invoked — which could lead to mistakes in future 
development.
   
   What do you think about doing a try-catch in `__del__`? I actually 
considered that beforehand too, but I was worried it could cause a silent fd 
leak if it fails.



-- 
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]

Reply via email to