potiuk commented on code in PR #40205:
URL: https://github.com/apache/airflow/pull/40205#discussion_r1639967290
##########
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:
I'd say:
* don't use executor by default at all (current behaviour)
* only use executor if `--use-executor` flag is passed (default will use
default, other values will use specific executor)
The ida behind `dag.test()` is that it should run fast - and
`__run_raw_task` does it. This is why for example in the past when we used
DebugExecutor, @dimberman contributed this `dag.test()` method - to avoid the
overhead and IMHO that should be default behaviour of `dags.test()` command -
independently of what executor is set locally via config or ENV var. Otherwise
it might be confusing for users who will run it with an environment that has
different configuration. That's why `--use-executor` flag should explicitly
indicate we want to use executor for this test run.
> I also can see an issue, especially related to
https://github.com/apache/airflow/pull/40010. Some options of dag.test() are
not compatible with executing with a different executor. For example, the
option mark-success-pattern in https://github.com/apache/airflow/pull/40010
would not work with a different executor because we are not using _run_task
Likely those two flags should be mutually exclusive, and that's also another
reason why `--use-executor` should be passed explicitly to trigger using
executor.
--
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]
