kaxil commented on code in PR #69735:
URL: https://github.com/apache/airflow/pull/69735#discussion_r3683801010
##########
providers/git/docs/bundles/index.rst:
##########
@@ -41,3 +41,25 @@ Example of using the GitDagBundle:
}
}
]'
+
+``tracking_ref`` accepts a branch, tag, or full commit SHA. Setting it to a
commit SHA pins the
+bundle to that exact commit:
+
+.. code-block:: bash
+
+ export AIRFLOW__DAG_PROCESSOR__DAG_BUNDLE_CONFIG_LIST='[
+ {
+ "name": "my-git-repo",
+ "classpath": "airflow.providers.git.bundles.git.GitDagBundle",
+ "kwargs": {
+ "repo_url": "https://github.com/org/repo.git";,
+ "tracking_ref": "a3d1850dd1aa1919a61620aa39f202185c9321c0",
+ "subdir": "dags"
+ }
+ }
+ ]'
+
+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.
Review 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](https://github.com/apache/airflow/blob/6af6a6a6bfe216618b0af4171bd5f1b10c83d14e/task-sdk/src/airflow/sdk/execution_time/task_runner.py#L1017))
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.
##########
providers/git/docs/bundles/index.rst:
##########
@@ -41,3 +41,25 @@ Example of using the GitDagBundle:
}
}
]'
+
+``tracking_ref`` accepts a branch, tag, or full commit SHA. Setting it to a
commit SHA pins the
+bundle to that exact commit:
+
+.. code-block:: bash
+
+ export AIRFLOW__DAG_PROCESSOR__DAG_BUNDLE_CONFIG_LIST='[
+ {
+ "name": "my-git-repo",
+ "classpath": "airflow.providers.git.bundles.git.GitDagBundle",
+ "kwargs": {
+ "repo_url": "https://github.com/org/repo.git";,
+ "tracking_ref": "a3d1850dd1aa1919a61620aa39f202185c9321c0",
+ "subdir": "dags"
+ }
+ }
+ ]'
+
+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.
Review 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](https://github.com/apache/airflow/blob/6af6a6a6bfe216618b0af4171bd5f1b10c83d14e/providers/git/src/airflow/providers/git/bundles/git.py#L215))
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.
##########
providers/git/tests/unit/git/bundles/test_git.py:
##########
@@ -701,6 +701,46 @@ def test_refresh_tag_moved_forward_and_backward(self,
mock_githook, git_repo):
files_in_repo = {f.name for f in bundle.path.iterdir() if f.is_file()}
assert {"test_dag.py"} == files_in_repo
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def test_tracking_ref_commit_sha_promote_and_rollback(self, mock_githook,
git_repo):
+ """Ensure tracking_ref accepts a full commit SHA, and a SHA-pinned
bundle can be
+ promoted to a new SHA and rolled back.
+
+ Promotion/rollback is simulated by creating a new bundle object with
the updated
+ tracking_ref, mirroring how a bundle config change is applied in
practice.
+ """
+ repo_path, repo = git_repo
+ mock_githook.return_value.repo_url = repo_path
+ first_commit = repo.head.commit
+
+ file_path = repo_path / "new_test.py"
+ with open(file_path, "w") as f:
+ f.write("hello world")
+ repo.index.add([file_path])
+ second_commit = repo.index.commit("Another commit")
Review 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.
--
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]
