dheerajturaga commented on code in PR #56206:
URL: https://github.com/apache/airflow/pull/56206#discussion_r2434093881
##########
providers/git/src/airflow/providers/git/bundles/git.py:
##########
@@ -109,48 +120,77 @@ def _initialize(self):
def initialize(self) -> None:
if not self.repo_url:
- raise AirflowException(f"Connection {self.git_conn_id} doesn't
have a host url")
+ raise RuntimeError(f"Connection {self.git_conn_id} doesn't have a
host url")
self._initialize()
super().initialize()
+ @retry(
+ retry=retry_if_exception_type((InvalidGitRepositoryError,
GitCommandError)),
+ stop=stop_after_attempt(2),
+ reraise=True,
+ )
def _clone_repo_if_required(self) -> None:
- if not os.path.exists(self.repo_path):
- self._log.info("Cloning repository", repo_path=self.repo_path,
bare_repo_path=self.bare_repo_path)
- try:
+ try:
+ if not os.path.exists(self.repo_path):
+ self._log.info(
+ "Cloning repository", repo_path=self.repo_path,
bare_repo_path=self.bare_repo_path
+ )
Repo.clone_from(
url=self.bare_repo_path,
to_path=self.repo_path,
)
- except NoSuchPathError as e:
- # Protection should the bare repo be removed manually
- raise AirflowException("Repository path: %s not found",
self.bare_repo_path) from e
- else:
- self._log.debug("repo exists", repo_path=self.repo_path)
- self.repo = Repo(self.repo_path)
-
+ else:
+ self._log.debug("repo exists", repo_path=self.repo_path)
+ self.repo = Repo(self.repo_path)
+ except NoSuchPathError as e:
+ # Protection should the bare repo be removed manually
+ raise RuntimeError("Repository path: %s not found",
self.bare_repo_path) from e
+ except (InvalidGitRepositoryError, GitCommandError) as e:
+ self._log.warning(
+ "Repository clone/open failed, cleaning up and retrying",
+ repo_path=self.repo_path,
+ exc=e,
+ )
+ if os.path.exists(self.repo_path):
+ shutil.rmtree(self.repo_path)
+ raise
+
+ @retry(
+ retry=retry_if_exception_type((InvalidGitRepositoryError,
GitCommandError)),
+ stop=stop_after_attempt(2),
+ reraise=True,
+ )
def _clone_bare_repo_if_required(self) -> None:
if not self.repo_url:
- raise AirflowException(f"Connection {self.git_conn_id} doesn't
have a host url")
- if not os.path.exists(self.bare_repo_path):
- self._log.info("Cloning bare repository",
bare_repo_path=self.bare_repo_path)
- try:
+ raise RuntimeError(f"Connection {self.git_conn_id} doesn't have a
host url")
+
+ try:
+ if not os.path.exists(self.bare_repo_path):
+ self._log.info("Cloning bare repository",
bare_repo_path=self.bare_repo_path)
Repo.clone_from(
url=self.repo_url,
to_path=self.bare_repo_path,
bare=True,
env=self.hook.env if self.hook else None,
)
- except GitCommandError as e:
- raise AirflowException("Error cloning repository") from e
- self.bare_repo = Repo(self.bare_repo_path)
+ self.bare_repo = Repo(self.bare_repo_path)
+ except (InvalidGitRepositoryError, GitCommandError) as e:
+ self._log.warning(
+ "Bare repository clone/open failed, cleaning up and retrying",
+ bare_repo_path=self.bare_repo_path,
+ exc=e,
+ )
+ if os.path.exists(self.bare_repo_path):
+ shutil.rmtree(self.bare_repo_path)
+ raise
def _ensure_version_in_bare_repo(self) -> None:
if not self.version:
return
if not self._has_version(self.bare_repo, self.version):
self._fetch_bare_repo()
if not self._has_version(self.bare_repo, self.version):
- raise AirflowException(f"Version {self.version} not found in
the repository")
+ raise RuntimeError(f"Version {self.version} not found in the
repository")
Review Comment:
@ephraimbuddy , I updated my PR to use RuntimeError instead in just the
parts I have touched. Hopefully all CI will turn green. If this doesn't pass CI
I would prefer to revert this commit and go back to my earlier implementation
and tackle the cleanup in another PR
--
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]
