ephraimbuddy commented on code in PR #56206:
URL: https://github.com/apache/airflow/pull/56206#discussion_r2431748184
##########
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:
Nope. I didn't ask you to change all the AirflowException to RuntimeError. I
only asked you to change the AirflowException to RuntimeError for the new ones
you raised. Please it's a breaking change to change all the exceptions
--
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]
