8silvergun commented on PR #72787: URL: https://github.com/apache/airflow/pull/72787#issuecomment-5613337302
Thanks for picking this up. One bit of context that may be useful before this is merged: #72759 was opened after noticing that #71535 appears to recover the same interrupted-clone state through a narrower path, and I already offered an explicit regression test for that case on #71535. The two approaches seem to differ mainly in recovery scope: * #71535 checks whether `tracking_ref` is resolvable and, only when it is not, fetches from the version clone's local bare `origin` before checkout. For the state in #72759, that should repopulate the missing refs without recreating the working clone. * This PR moves checkout into `_clone_repo_if_required()`'s existing retry/cleanup block, so any `GitCommandError` during checkout removes the working clone and retries from the healthy bare mirror. That also fixes #71388, but is a broader behavior change because checkout failures that are not caused by an interrupted clone will also trigger cleanup/reclone. The regression test added here is exactly the kind of coverage I was hoping to preserve for #72759, so I think the test itself is valuable either way. Would a maintainer mind taking a quick look at #71535 and this PR together and deciding which recovery behavior is preferable? If the broader cleanup/reclone semantics are intentional, this PR covers #72759 directly. If the narrower fetch-before-checkout behavior is preferred, the interrupted-clone regression test could instead be added to #71535 so this failure mode remains explicitly covered. -- 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]
