Document commit SHA support for GitDagBundle tracking_ref - #69735
Document commit SHA support for GitDagBundle tracking_ref#69735coleheflin wants to merge 6 commits into
Conversation
GitDagBundle.tracking_ref already works with a full commit SHA in practice (checkout, promote, and rollback), but the provider docs only described it as a branch or tag. Clarify the docstring and provider docs, add a commit-SHA example, note static-ref refresh behavior, and add test coverage for SHA-pinned tracking_ref. Co-Authored-By: Otto <noreply@astronomer.io>
Unlike moving a branch or tag ref, repointing tracking_ref to a new commit SHA is a dag_bundle_config_list config change, so it only takes effect after the Dag processor and workers are restarted to reload the configuration. Co-Authored-By: Otto <noreply@astronomer.io>
Clarify that promoting/rolling back a SHA-pinned tracking_ref (an environment-level config change) is a different mechanism from Dag bundle versioning, which lets individual runs be rerun with their original commit regardless of the bundle's current tracking_ref. Co-Authored-By: Otto <noreply@astronomer.io>
The promote/rollback test already exercises basic SHA checkout as its first step, so the standalone test was redundant. Also tighten the docs note down to two paragraphs without dropping any of the facts. Co-Authored-By: Otto <noreply@astronomer.io>
The rerun_with_latest_version distinction is a separate feature and isn't needed to document tracking_ref's SHA support; keep the note focused on the restart caveat. Co-Authored-By: Otto <noreply@astronomer.io>
…otion The doc previously said promoting or rolling back a SHA-pinned tracking_ref requires restarting both the Dag processor and workers, but only the Dag processor needs to be restarted to pick up the change. Co-Authored-By: Otto <noreply@astronomer.io>
99bc2f2 to
6af6a6a
Compare
|
@potiuk this is ready for review, CI is fully green now! One piece of feedback on the triage flow: the maintainer note was added directly into the PR description rather than as a comment, so I didn't get a GitHub notification and didn't realize there was action needed on my end until I checked back in. Posting maintainer triage notes as a regular PR comment (with a tag) would make sure they surface in notifications/email inbox. Thanks for the help getting this triaged! |
| Branches move as new commits are pushed, so combined with ``refresh_interval`` they pick up new code | ||
| without a restart. Tags and commit SHAs are static (assuming tags aren't moved), pinning the bundle | ||
| to known-good code — but promoting or rolling back a SHA means changing ``tracking_ref`` in | ||
| ``dag_bundle_config_list`` itself, which requires restarting the Dag processor to take effect. |
There was a problem hiding this comment.
Promoting forward doesn't take effect on a restart alone if the bundle storage path survives that restart, which is the default. _initialize runs self.repo.git.checkout(self.tracking_ref) (git.py:215) before the working repo has fetched anything: _fetch_bare_repo updates the bare mirror, but tracking_repo is only fetched inside refresh(), which runs after that checkout. A tracking_repo cloned while the old SHA was HEAD has no objects for the new one, so initialize() raises GitCommandError: fatal: reference is not a tree: <new sha>.
I ran this against a real bundle: promoting to a commit created after the first initialize() fails, promoting to a tag created after it fails the same way (pathspec 'v2' did not match), rolling back to an older SHA works because the objects are already local, and a fresh storage path works. Since dag_bundle_storage_path defaults to /tmp/airflow/dag_bundles, a plain Dag processor restart on the same host keeps the stale clone and stays broken until the bundle directory is removed. On Kubernetes it depends on whether that path sits on a volume.
Could you scope this sentence, so it doesn't promise that promoting works after a restart? Something like: rollback works, promoting needs the bundle storage cleared (fresh pod, or deleting the bundle dir). The real fix is fetching before the checkout in _initialize, which deserves its own PR rather than blocking this one.
| Branches move as new commits are pushed, so combined with ``refresh_interval`` they pick up new code | ||
| without a restart. Tags and commit SHAs are static (assuming tags aren't moved), pinning the bundle | ||
| to known-good code — but promoting or rolling back a SHA means changing ``tracking_ref`` in | ||
| ``dag_bundle_config_list`` itself, which requires restarting the Dag processor to take effect. |
There was a problem hiding this comment.
Is the Dag processor enough in every case? With [dag_processor] disable_bundle_versioning (or DAG(disable_bundle_versioning=True)) no bundle_version is stored on the dag run, so the worker calls DagBundlesManager().get_bundle(name, version=None) (task_runner.py:1017) and resolves code from its own tracking_ref. A worker still holding the old SHA in its dag_bundle_config_list would run the old commit. The PR description's original wording ("Dag processor and workers") covered that, so maybe scope this to versioning being enabled and mention the worker case.
| with open(file_path, "w") as f: | ||
| f.write("hello world") | ||
| repo.index.add([file_path]) | ||
| second_commit = repo.index.commit("Another commit") |
There was a problem hiding this comment.
second_commit is created before the first initialize(), so both the bare mirror and the tracking_repo clone already contain it, and the promote step never exercises what a real config change hits. Moving these four lines below the first bundle.initialize() turns it into a genuine promote, and the test then fails with fatal: reference is not a tree: <sha> from the checkout in _initialize. Worth doing even though it goes red, because the docstring says this mirrors "how a bundle config change is applied in practice" and as written it doesn't. See my note on the docs paragraph for the mechanism.
3 participants
Clarifies that
GitDagBundle.tracking_refsupports a branch, tag, or full commit SHA — it already works with a SHA in practice, but the provider docs only described branch/tag. Adds a commit-SHA example to the bundle docs, a note on static-ref refresh behavior (a SHA-pinned bundle doesn't pick up new commits viarefresh_interval; promoting/rolling back requires updatingtracking_refin the bundle config), and test coverage for SHA checkout, promotion, and rollback.Verified manually against a live repository: setting
tracking_refto a full commit SHA checks out that exact commit, promotes to a new SHA when the bundle config is updated, and rolls back when reverted to the prior SHA. The added tests cover the same scenarios. Note that (unlike moving a branch/tag ref) changing a pinned SHA is adag_bundle_config_listconfig change, so it requires restarting the Dag processor and workers to take effect.Was generative AI tooling used to co-author this PR?
Generated-by: Otto following the guidelines
🤖 Generated by Otto
Important
🛠️ Maintainer triage note for @coleheflin · by
@potiuk· 2026-07-28 16:10 UTCHelpful heads-up from the maintainers — please address before this PR can be reviewed:
The ball is in your court — you've been assigned to this PR. Fix the above, then mark it Ready for review.
See the Pull Request quality criteria for how to fix each item. There is no rush.
Note: your branch is 345 commits behind
main— please rebase and push again to get up-to-date CI results.Automated triage — may be imperfect; a maintainer takes the next look. We use this two-stage triage process so maintainers' limited time goes to the conversation with you.