uranusjr commented on code in PR #28900:
URL: https://github.com/apache/airflow/pull/28900#discussion_r1262193030
##########
airflow/models/dag.py:
##########
@@ -1378,6 +1379,37 @@ def normalized_schedule_interval(self) ->
ScheduleInterval:
_schedule_interval = self.schedule_interval
return _schedule_interval
+ @staticmethod
+ @internal_api_call
+ @provide_session
+ def fetch_callback(
+ dag: DAG,
+ dagrun: DagRun | DagRunPydantic,
+ success: bool = True,
+ reason: str | None = None,
+ session: Session = NEW_SESSION,
+ ):
Review Comment:
```suggestion
*,
session: Session = NEW_SESSION,
) -> None | tuple[TaskStateChangeCallback |
list[TaskStateChangeCallback], Context]:
```
##########
airflow/models/baseoperator.py:
##########
@@ -907,7 +907,7 @@ def __init__(
max_active_tis_per_dag = task_concurrency
self.max_active_tis_per_dag: int | None = max_active_tis_per_dag
self.max_active_tis_per_dagrun: int | None = max_active_tis_per_dagrun
- self.do_xcom_push = do_xcom_push
+ self.do_xcom_push: bool = do_xcom_push
Review Comment:
Is this a Mypy issue? (Just asking since it doesn’t seem to be relevant to
anything else)
##########
airflow/dag_processing/processor.py:
##########
@@ -745,25 +746,33 @@ def execute_callbacks_without_dag(
@provide_session
def _execute_dag_callbacks(self, dagbag: DagBag, request:
DagCallbackRequest, session: Session):
dag = dagbag.dags[request.dag_id]
- dag_run = dag.get_dagrun(run_id=request.run_id, session=session)
- dag.handle_callback(
- dagrun=dag_run, success=not request.is_failure_callback,
reason=request.msg, session=session
+ dagrun = DAG.fetch_dagrun(dag_id=dag.dag_id, run_id=request.run_id,
session=session)
+ callbacks, context = (
+ DAG.fetch_callback(
+ dag=dag,
+ dagrun=dagrun,
+ success=not request.is_failure_callback,
+ reason=request.msg,
+ session=session,
+ )
+ or None,
+ None,
)
Review Comment:
This looks wrong? If `fetch_callback` returns something, it is alreay a
tuple and this would incorrectly result in a nested structure
##########
airflow/models/baseoperator.py:
##########
@@ -907,7 +907,7 @@ def __init__(
max_active_tis_per_dag = task_concurrency
self.max_active_tis_per_dag: int | None = max_active_tis_per_dag
self.max_active_tis_per_dagrun: int | None = max_active_tis_per_dagrun
- self.do_xcom_push = do_xcom_push
+ self.do_xcom_push: bool = do_xcom_push
Review Comment:
Is this a Mypy issue? (Just asking since it doesn’t seem to be relevant to
anything else)
--
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]
