Add dag_bundle_name option to subprocess coordinators - #70805
Add dag_bundle_name option to subprocess coordinators#70805jason810496 wants to merge 10 commits into
Conversation
1b76bfe to
182919a
Compare
bc97b03 to
12de628
Compare
12de628 to
c7ab92a
Compare
But the signature still changed, a new |
47285a1 to
30c1fe8
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends Task SDK subprocess coordinators (Java/Node/Executable) to support locating compiled artifacts via Dag bundles, by adding an optional dag_bundle_name option to the SubprocessCoordinator base and centralizing bundle materialization + access checks in a new shared module.
Changes:
- Add artifact source resolution to
SubprocessCoordinator(explicit root vs named Dag bundle vs task’s own bundle), expose resolved scan roots via_get_scan_roots(), and holdBundleVersionLockwhile the subprocess runs. - Split bundle initialization/access verification out of
task_runnerintoairflow.sdk.execution_time.bundles, and split tracing helpers intoairflow.sdk.execution_time.tracing. - Add a cached “bundle config snapshot” in
DagBundlesManager(plus test/fixture cache clearing) and update docs/tests for the new configuration behavior.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| task-sdk/src/airflow/sdk/coordinators/_subprocess.py | Implements artifact source classification, root resolution, bundle pinning, and lock handling for subprocess coordinators. |
| task-sdk/src/airflow/sdk/coordinators/java/coordinator.py | Switches Java artifact scanning to use base-resolved scan roots; makes jars_root optional. |
| task-sdk/src/airflow/sdk/coordinators/node/coordinator.py | Switches Node artifact scanning to use base-resolved scan roots; makes bundles_root optional. |
| task-sdk/src/airflow/sdk/coordinators/executable/coordinator.py | Switches executable bundle scanning to use base-resolved scan roots; makes executables_root optional. |
| task-sdk/src/airflow/sdk/execution_time/bundles.py | New shared bundle materialization + access-check helper used by task runner and subprocess coordinators. |
| task-sdk/src/airflow/sdk/execution_time/tracing.py | New shared tracing helper module (detail_span). |
| task-sdk/src/airflow/sdk/execution_time/task_runner.py | Uses initialize_ti_bundle() and imports detail_span from the new tracing module. |
| task-sdk/tests/task_sdk/coordinators/test_subprocess.py | Adds tests for artifact source classification, root resolution, locking, and scan-root wiring. |
| task-sdk/tests/task_sdk/coordinators/java/test_coordinator.py | Updates Java coordinator tests for optional roots and scan-root wiring. |
| task-sdk/tests/task_sdk/coordinators/node/test_coordinator.py | Updates Node coordinator tests for optional roots and scan-root wiring. |
| task-sdk/tests/task_sdk/coordinators/executable/test_coordinator.py | Updates executable coordinator tests for optional roots and scan-root wiring. |
| task-sdk/tests/task_sdk/execution_time/test_bundles.py | New tests for initialize_ti_bundle() and verify_bundle_access(). |
| task-sdk/tests/task_sdk/execution_time/test_tracing.py | New focused tests for detail_span behavior at different detail levels. |
| task-sdk/tests/task_sdk/execution_time/test_task_runner.py | Removes relocated tests; updates tracer patch points to the new tracing module. |
| airflow-core/src/airflow/dag_processing/bundles/manager.py | Adds cached bundle-config snapshot and DagBundlesManager.is_bundle_configured(). |
| airflow-core/tests/unit/dag_processing/bundles/test_dag_bundle_manager.py | Tests is_bundle_configured() behavior and clears the snapshot cache between tests. |
| devel-common/src/tests_common/test_utils/config.py | Clears the Dag bundle config snapshot cache when conf_vars mutates config. |
| devel-common/src/tests_common/pytest_plugin.py | Adds autouse fixture to clear Dag bundle config snapshot cache between tests. |
| generated/known_airflow_exceptions.txt | Updates known AirflowException locations due to function relocation. |
| airflow-core/tests/integration/otel/test_otel.py | Updates expected span name after renaming _verify_bundle_access → verify_bundle_access. |
| contributing-docs/30_new_language_sdk.rst | Documents _get_scan_roots() as the correct way for coordinators to access resolved roots. |
| airflow-core/docs/authoring-and-scheduling/language-sdks/java.rst | Documents dag_bundle_name and new root selection rules for Java coordinator. |
| airflow-core/docs/authoring-and-scheduling/language-sdks/typescript.rst | Documents dag_bundle_name and new root selection rules for TypeScript coordinator. |
| airflow-core/docs/authoring-and-scheduling/language-sdks/go.rst | Documents dag_bundle_name and new root selection rules for Go/executable coordinator. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
30c1fe8 to
b148880
Compare
Sorry for oversight this comment, I just addressed in 947b6dd by introducing |
Language-SDK coordinators previously required a hand-managed filesystem root (jars_root/executables_root/bundles_root) for compiled artifacts, kept outside Airflow's Dag-bundle machinery and prone to drifting from the Dags they serve. Allowing artifacts to be located through a configured Dag bundle -- or the task's own bundle when none is named -- lets them be delivered and versioned the same way as everything else.
Language-SDK subprocess coordinators materialized artifact bundles without the version-lock protection the Python task path holds, so bundle cleanup could delete a version a running JVM was still reading from. Resolution now holds the version lock for the subprocess lifetime, mirroring the Python task path, and materialization failures surface in the task log rather than the worker log. Classification moves to the base coordinator so a subclass that adds no explicit-root kwarg falls back to the task-bundle default instead of failing every task, the Java coordinator now requires an explicit entrypoint when it has to scan a whole bundle, and configured-bundle lookup checks names only so an unrelated unimportable bundle no longer breaks coordinator construction.
When a subprocess coordinator locates artifacts through a Dag bundle rather than an explicit root, JavaCoordinator previously rejected a missing main_class outright. That made the co-located default unusable without extra configuration and left the entrypoint no more determinate than an auto-detected scan would be. Auto-detect over the resolved bundle now handles that case, with the remaining cross-bundle ambiguity deferred to an IMPORT_ERROR-stage check under AIP-85. A named Dag bundle resolves to its latest version, so its version was unset and the version lock held around the subprocess silently did nothing, letting bundle cleanup delete artifacts a running task was still reading. The concrete current version is now pinned before locking.
A language-SDK task start needs a bundle materialized on disk before the subprocess can be launched, but reaching that through the task runner made every such start pay for the operator tree, pydantic, and OTel plumbing that the supervisor side never uses. The bundle lifecycle now lives on its own, so the coordinator depends on the Dag-bundle machinery alone. The bundle-name check a coordinator runs at construction and the class-importing parse the manager runs at startup previously read the configuration independently, so the two could disagree about what is configured. Keying the parsed configuration on the configuration itself lets both share one snapshot while a configuration change is still picked up. Subclasses had to declare the name of their explicit-root kwarg separately from its value, which is two things to keep in step for one piece of information.
Splitting the read across a keyed builder and a wrapper meant two names for one idea, and the empty case leaked out as a module-level constant that existed only to be returned. One function now answers "what is configured", parsed once per process so both readers of it see the same answer. Parsing once per process means a later configuration change is not observed. That only arises in tests, which switch configuration freely, so overriding configuration there drops the memo rather than silently serving a stale one.
External language SDK coordinators can implement the documented command hook. Keeping that contract stable avoids unexpected-keyword failures when upgrading while allowing Dag-bundle-backed artifact discovery.
Module-level imports keep dependency analysis reliable and follow Airflow's import-placement contract without changing bundle access behavior.
b148880 to
0938f4b
Compare
Bundle resolution does not need mutable instance state, while overwriting scan roots can make a command build against another task's artifacts.
An explicitly empty root is likely a configuration or templating error. Treating it as omitted can silently scan the task's Dag bundle and run unintended artifacts.
jason810496
left a comment
There was a problem hiding this comment.
Thanks for the review.
Why
Language-SDK subprocess coordinators (Java, Executable, Node) required a hand-managed filesystem root (
jars_root/executables_root/bundles_root) for compiled artifacts. Users should be able to deploy those artifacts by existing DagBundle mechanism as well.How
dag_bundle_namekwarg on theSubprocessCoordinatorbase, giving three ways to locate artifacts:dag_bundle_nameset: scan that configured bundle's path._get_scan_roots()while keeping the existing_build_execute_task_command(what)hook signature, so a subclass only declares its explicit-root kwarg through_explicit_artifact_roots.airflow.sdk.execution_time.bundles.initialize_ti_bundle, a new module split out oftask_runnerso starting a language-SDK task does not import the task runner into the supervisor.dag_bundle_nameare not both set, and that a named bundle is configured, via a newDagBundlesManager.is_bundle_configuredthat reads configured names without importing any bundle class.Notes
bundle_info.versionbecause it is the same artifact the run was created from; a separately named artifact bundle resolves to the version current when the task starts. Either way the bundle is materialized at a concrete version, soBundleVersionLockprotects the same tree the subprocess scans.Was generative AI tooling used to co-author this PR?