kaxil commented on code in PR #67217:
URL: https://github.com/apache/airflow/pull/67217#discussion_r3455807023
##########
task-sdk/src/airflow/sdk/execution_time/callback_supervisor.py:
##########
@@ -227,6 +227,7 @@ def _target():
bundle = DagBundlesManager().get_bundle(
name=bundle_info.name,
version=bundle_info.version,
+ version_data=bundle_info.version_data,
Review Comment:
Two things worth addressing imo:
1. The producer paths don't populate `version_data`.
`ExecuteCallback.make()` in
`airflow-core/src/airflow/executors/workloads/callback.py:117-120` builds
`BundleInfo(name=..., version=...)` with no `version_data`, and
`_resolve_callback_bundle` at
`airflow-core/src/airflow/dag_processing/manager.py:693` calls
`get_bundle(name=request.bundle_name, version=request.bundle_version)` without
it. `BaseCallbackRequest` (`callback_requests.py:48`) also doesn't carry the
field. Net effect for a pinned run with an S3-manifest-style bundle: tasks
initialize the bundle with the manifest; callbacks for the same `dag_run.id`
initialize without it. Worth passing `version_data` through
`ExecuteCallback.make()` + `_resolve_callback_bundle` + `BaseCallbackRequest`.
2. The `_BundleInfoLike` Protocol just above (around line 67-69) doesn't
declare `version_data`. This file uses it specifically because mypy treats core
and SDK `BundleInfo` as distinct types, so an external duck-typer satisfying
the Protocol will type-check today but trip an `AttributeError` here that the
surrounding `except Exception:` will swallow into a log warning. Worth adding
`version_data: dict[str, Any] | None` to the Protocol (plus an `Any` import
under `TYPE_CHECKING`).
--
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]
