vincbeck commented on code in PR #40205:
URL: https://github.com/apache/airflow/pull/40205#discussion_r1639958515


##########
airflow/models/dag.py:
##########
@@ -2952,6 +2952,21 @@ def add_logger_if_needed(ti: TaskInstance):
             # Instead of starting a scheduler, we run the minimal loop 
possible to check
             # for task readiness and dependency management. This is notably 
faster
             # than creating a BackfillJob and allows us to surface logs to the 
user
+
+            from airflow.executors.local_executor import LocalExecutor
+
+            # Fetch the executor from config
+            # ``Dag.test()`` works in two different modes depending on the 
executor:
+            # - if the executor is ``LocalExecutor``, runs the task locally 
using ``_run_task``
+            # - if the executor is not ``LocalExecutor``, sends the task 
instances to the executor with
+            #   ``BaseExecutor.queue_task_instance``

Review Comment:
   > keep dags.test() running always using _run_raw_task
   
   Only if the executor is `LocalExecutor` right? If an executor is passed 
using `use-executor`, it should be using `executor.queue_task_instance` right? 
I just want to confirm we are aligned
   
   I also can see an issue, especially related to #40010. Some options of 
`dag.test()` are not compatible with executing with a different executor. For 
example, the option `mark-success-pattern` in #40010 would not work with a 
different executor because we are not using `_run_task`



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to