hkc-8010 opened a new pull request, #72923:
URL: https://github.com/apache/airflow/pull/72923
`GitDagBundle` clones the working repository straight into its final
`repo_path`. A process
killed part way through that clone leaves a directory that git can open but
that cannot check
out the tracking ref, and because `_clone_repo_if_required` only clones when
`repo_path` does
not exist, every later initialization reuses it and fails. On a long-lived
worker, where the
bundle directory is pod-local and lives as long as the pod, that turns a
single interrupted
clone into a permanently broken worker.
Seen in production on a worker whose clone was killed by SIGTERM 315 ms in,
after a task was
marked failed from the UI. Every one of the 30 tasks that landed on that pod
afterwards failed
at parse time with:
```
git checkout master
error: The following untracked working tree files would be overwritten by
checkout:
.agents/instructions/airflow-patterns.md
<...>
Aborting
```
## Change
Clone into a staging directory next to `repo_path` and `os.replace` it into
place once the
clone (and any sparse-checkout setup) is complete. `rename` is atomic, so
`repo_path` either
does not exist or holds a finished clone. An interrupted clone now leaves
only an orphaned
staging directory, which the next attempt removes before it starts.
This is deliberately the prevention half only. Recovering an already-broken
`repo_path` is
#72787, which moves the checkout inside `_clone_repo_if_required`'s retry
and `rmtree` block,
and I would rather that landed than have a competing PR for it. The two
compose: with both,
a directory cannot get poisoned in the first place, and one poisoned by an
older provider
version still gets discarded and re-cloned. #72759 also names a residual
case that recovery
alone does not reach, where the clone dies before the remote configuration
is written and
`self.repo.remotes.origin` raises `AttributeError`; staging removes that one
too, because
nothing lands at `repo_path` until the clone is finished.
Note for whoever merges second: #72787 and this PR both touch the clone
block, so one will
need a trivial rebase. There is no semantic conflict.
## Tests
- `test_interrupted_clone_leaves_no_working_repo`: patches `Repo.clone_from`
to write a `.git`
skeleton into its target and then raise, which is the state a real `git
clone` leaves when
it is killed before it writes refs or the working tree. Asserts
`repo_path` does not exist
afterwards and that the next `initialize()` produces a working bundle. On
main the first
assertion fails (`assert not True`) because the half-written directory is
left at
`repo_path`.
- `test_stale_staging_directory_is_discarded`: a staging directory left by a
clone that had no
chance to clean up does not block the next one.
- Two existing tests that mock `Repo` wholesale needed their `clone_from` to
create its target
directory, which is what a real clone does. Factored that into
`_clone_from_creating_target`.
Validation:
- `providers/git/tests/unit/git/bundles/test_git.py` - 107 passed locally
(105 before this PR).
- `breeze testing providers-tests --backend sqlite --python 3.10 --db-reset`
on the same file -
107 passed.
- Both new tests confirmed failing with the source file reverted to
`upstream/main`.
- `prek` on both changed files and `mypy` on `git.py` are clean.
No newsfragment: provider bugfix, no user-facing API change.
closes: #72922
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
