o-nikolas commented on code in PR #45631:
URL: https://github.com/apache/airflow/pull/45631#discussion_r1915825948


##########
airflow/utils/log/file_task_handler.py:
##########
@@ -313,10 +313,18 @@ def _render_filename(self, ti: TaskInstance, try_number: 
int, session=NEW_SESSIO
     def _read_grouped_logs(self):
         return False
 
-    @cached_property
-    def _executor_get_task_log(self) -> Callable[[TaskInstance, int], 
tuple[list[str], list[str]]]:
-        """This cached property avoids loading executor repeatedly."""
-        executor = ExecutorLoader.get_default_executor()
+    def _get_executor_get_task_log(

Review Comment:
   You've dropped the cached property of this (which you kind of have to now) 
but we need to replace it with something else otherwise a new executor is going 
to get constructed every time this is run. I was just working on this until I 
went through my email and noticed this one!
   
   Here is a snippet of what I was playing around with:
   ```python
   def _executor_get_task_log(
       self, ti: TaskInstance
   ) -> Callable[[TaskInstance, int], tuple[list[str], list[str]]]:
       executor = str(ti.executor)
       if not self.executor_instances[executor]:
           # We have not yet loaded this executor, so load it now
           if not executor:
               # The ti is not specifying an executor, so fetch the default
               self.executor_instances[executor] = 
ExecutorLoader.get_default_executor()
           else:
               self.executor_instances[executor] = 
ExecutorLoader.load_executor(executor)
   
       return self.executor_instances[executor].get_task_log
   
   ```
   The creation of the `executor_instances` in `init` isn't show here but 
that's obviously required.
   
   Does that make sense?



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