uranusjr commented on code in PR #31001:
URL: https://github.com/apache/airflow/pull/31001#discussion_r1209696998


##########
airflow/executors/celery_executor_utils.py:
##########


Review Comment:
   Maybe we should add a comment or docstring somewhere in this module to warn 
future contributors against importing this anywhere global?



##########
airflow/executors/celery_executor.py:
##########
@@ -250,7 +119,7 @@ def _num_tasks_per_send_process(self, to_send_count: int) 
-> int:
         return max(1, int(math.ceil(1.0 * to_send_count / 
self._sync_parallelism)))
 
     def _process_tasks(self, task_tuples: list[TaskTuple]) -> None:
-        task_tuples_to_send = [task_tuple[:3] + (execute_command,) for 
task_tuple in task_tuples]
+        task_tuples_to_send = [task_tuple[:3] + (self.execute_command,) for 
task_tuple in task_tuples]

Review Comment:
   Why not just import `execute_command` in this function?



##########
airflow/executors/celery_executor.py:
##########
@@ -64,151 +58,23 @@
     TaskInstanceInCelery = Tuple[TaskInstanceKey, CommandType, Optional[str], 
Task]
 
 
-log = logging.getLogger(__name__)
+# PEP562
+def __getattr__(name):
+    # This allows us to make the Celery app accessible through the
+    # celery_executor module without the time cost of it's import and

Review Comment:
   ```suggestion
       # celery_executor module without the time cost of its import and
   ```



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