Skip to content

Commit ef66a9f

Browse files
Handle some git exceptions when initializing GitDagBundle (apache#46253)
* Handle some git exceptions when initializing GitDagBundle Permission error raises GitCommandError, and if the cloned path is manually removed, it will result in a path not found error when cloning from the bare repo path * Apply suggestions from code review Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> * fixup! Apply suggestions from code review * Fix comment * Update airflow/dag_processing/bundles/git.py Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com> * fix test --------- Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
1 parent 8a3757b commit ef66a9f

File tree

2 files changed

+47
-11
lines changed

2 files changed

+47
-11
lines changed

airflow/dag_processing/bundles/git.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from urllib.parse import urlparse
2424

2525
from git import Repo
26-
from git.exc import BadName
26+
from git.exc import BadName, GitCommandError, NoSuchPathError
2727

2828
from airflow.dag_processing.bundles.base import BaseDagBundle
2929
from airflow.exceptions import AirflowException
@@ -157,10 +157,14 @@ def initialize(self) -> None:
157157
def _clone_repo_if_required(self) -> None:
158158
if not os.path.exists(self.repo_path):
159159
self.log.info("Cloning repository to %s from %s", self.repo_path, self.bare_repo_path)
160-
Repo.clone_from(
161-
url=self.bare_repo_path,
162-
to_path=self.repo_path,
163-
)
160+
try:
161+
Repo.clone_from(
162+
url=self.bare_repo_path,
163+
to_path=self.repo_path,
164+
)
165+
except NoSuchPathError as e:
166+
# Protection should the bare repo be removed manually
167+
raise AirflowException("Repository path: %s not found", self.bare_repo_path) from e
164168

165169
self.repo = Repo(self.repo_path)
166170

@@ -169,12 +173,15 @@ def _clone_bare_repo_if_required(self) -> None:
169173
raise AirflowException(f"Connection {self.git_conn_id} doesn't have a host url")
170174
if not os.path.exists(self.bare_repo_path):
171175
self.log.info("Cloning bare repository to %s", self.bare_repo_path)
172-
Repo.clone_from(
173-
url=self.repo_url,
174-
to_path=self.bare_repo_path,
175-
bare=True,
176-
env=self.hook.env,
177-
)
176+
try:
177+
Repo.clone_from(
178+
url=self.repo_url,
179+
to_path=self.bare_repo_path,
180+
bare=True,
181+
env=self.hook.env,
182+
)
183+
except GitCommandError as e:
184+
raise AirflowException("Error cloning repository") from e
178185
self.bare_repo = Repo(self.bare_repo_path)
179186

180187
def _ensure_version_in_bare_repo(self) -> None:

tests/dag_processing/test_dag_bundles.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@
1717

1818
from __future__ import annotations
1919

20+
import re
2021
import tempfile
2122
from pathlib import Path
2223
from unittest import mock
2324

2425
import pytest
2526
from git import Repo
27+
from git.exc import GitCommandError, NoSuchPathError
2628

2729
from airflow.dag_processing.bundles.base import BaseDagBundle
2830
from airflow.dag_processing.bundles.git import GitDagBundle, GitHook
@@ -477,3 +479,30 @@ def test_view_url_returns_none_when_no_version_in_view_url(self, mock_gitrepo):
477479
)
478480
view_url = bundle.view_url(None)
479481
assert view_url is None
482+
483+
@mock.patch("airflow.dag_processing.bundles.git.GitHook")
484+
def test_clone_bare_repo_git_command_error(self, mock_githook):
485+
mock_githook.return_value.repo_url = "git@github.com:apache/airflow.git"
486+
mock_githook.return_value.env = {}
487+
488+
with mock.patch("airflow.dag_processing.bundles.git.Repo.clone_from") as mock_clone:
489+
mock_clone.side_effect = GitCommandError("clone", "Simulated error")
490+
bundle = GitDagBundle(name="test", tracking_ref="main")
491+
with pytest.raises(
492+
AirflowException,
493+
match=re.escape("Error cloning repository"),
494+
):
495+
bundle.initialize()
496+
497+
@mock.patch("airflow.dag_processing.bundles.git.GitHook")
498+
def test_clone_repo_no_such_path_error(self, mock_githook):
499+
mock_githook.return_value.repo_url = "git@github.com:apache/airflow.git"
500+
501+
with mock.patch("airflow.dag_processing.bundles.git.os.path.exists", return_value=False):
502+
with mock.patch("airflow.dag_processing.bundles.git.Repo.clone_from") as mock_clone:
503+
mock_clone.side_effect = NoSuchPathError("Path not found")
504+
bundle = GitDagBundle(name="test", tracking_ref="main")
505+
with pytest.raises(AirflowException) as exc_info:
506+
bundle._clone_repo_if_required()
507+
508+
assert "Repository path: %s not found" in str(exc_info.value)

0 commit comments

Comments
 (0)