potiuk commented on code in PR #38563:
URL: https://github.com/apache/airflow/pull/38563#discussion_r1553938903
##########
airflow/cli/commands/task_command.py:
##########
@@ -278,7 +279,15 @@ def _run_task_by_local_task_job(args, ti: TaskInstance |
TaskInstancePydantic) -
external_executor_id=_extract_external_executor_id(args),
)
try:
- ret = run_job(job=job_runner.job, execute_callable=job_runner._execute)
+ # If internal API is used, we must pass session=None so that one is
not created
+ # by the provide_session decorator. All downstream functions that
actually use
+ # a session are already set up for RPC
+ # todo: perhaps instead we should simply modify the provide_session
decorator
+ # to *not* provide a session when using internal API
Review Comment:
I'd say provide_session should provide a fake session that should:
1) store traceback where it has been used to provide session for the first
time
2) raise an exception when it is accessed for anything explaining both:
a) where it has been provided
b) where it has been used
c) eventually explanation what to do to contributor.
Both (in CI / breeze / development environment) using `rich`'s traceback
functionality (https://rich.readthedocs.io/en/stable/traceback.html).
I see this as CI/Development/Diagnostic tool. If we return None, the most
we will get is an exception that None has missing property - which willl be
difficult to really trace, because the session can be passed throuh multiple
layers and multiple method calls before it is used.
When we enable internal API, there will be a test in CI failing if somone
will add a new DB call and did not test it with internal API, so what we really
want in this case is to provide the fastest path for the contributor to find
out what they should do to fix the problem. I don't think passing None is good
enough for that.
--
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
