hwang-cadent commented on PR #56973:
URL: https://github.com/apache/airflow/pull/56973#issuecomment-4399147348
@potiuk @dabla — addressed the review feedback in `93beb9697f` and split out
the serialization concern as you suggested:
**Comments 1, 2 and 4** (in `93beb9697f` on this PR):
1. **AF3 architecture inconsistency** — Moved `XCOM_DAG_ID` push out of
`_trigger_dag_af_3()` and into `task_runner._handle_trigger_dag_run()` next to
`trigger_run_id`. Both XComs are now written together only after a successful
trigger, so XCom state stays consistent if the trigger fails.
2. **`get_link` fallback check** — Changed `if not trigger_dag_id:` to `if
trigger_dag_id is None:` so any explicit value pushed to XCom is honored.
4. **Ordered call assertions** — Restored `assert_has_calls` with the full
ordered call list in `test_handle_trigger_dag_run_wait_for_completion` (and
added the new `SetXCom("trigger_dag_id")` to `test_handle_trigger_dag_run`).
**Comment 3** (separate PR):
3. **Bundled serialization fix** — Agreed. Split out as a standalone PR:
**#66564** (`Fix ARG_NOT_SET deserialization for `_date` fields in
OperatorSerialization`). Once that lands I'll rebase this PR on `main` and drop
the `serialized_objects.py` / `test_serialized_objects.py` hunks from here.
Could you take another look when you get a chance? Thank you!
--
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]
