refactor: Exchange NotADirectoryError for better exception
This patch set replaces raising NotADirectoryError after trying to parse a repository for its root path (normalize_repo_path in pegleg.engine.util.git) with a better exception (exceptions.GitInvalidRepoException). It is better because a folder can still not be a repo, so raising the first exception isn't apropos. Next, this patch set changes where the exception is raised -- which is in normalize_repo_path itself, which is more appropriate as the function is used in many places and so there should be intrinsic error handling so as to avoid having to wrap it every time. Change-Id: I918d8c293f1140eb80c83499dba2c23af232b79e
This commit is contained in:
parent
c498bfee81
commit
2e51779d57
|
@ -56,3 +56,8 @@ Git Exceptions
|
|||
:members:
|
||||
:show-inheritance:
|
||||
:undoc-members:
|
||||
|
||||
.. autoexception:: pegleg.engine.exceptions.GitInvalidRepoException
|
||||
:members:
|
||||
:show-inheritance:
|
||||
:undoc-members:
|
||||
|
|
|
@ -81,4 +81,9 @@ class GitSSHException(BaseGitException):
|
|||
|
||||
class GitConfigException(BaseGitException):
|
||||
"""Exception that occurs when reading Git repo config fails."""
|
||||
message = ("Failed to read Git config file for repo_url=%(repo_url)s")
|
||||
message = ("Failed to read Git config file for repo path: %(repo_path)s")
|
||||
|
||||
|
||||
class GitInvalidRepoException(BaseGitException):
|
||||
"""Exception raised when an invalid repository is detected."""
|
||||
message = ("The repository path or URL is invalid: %(repo_path)s")
|
||||
|
|
|
@ -17,7 +17,6 @@ import os
|
|||
import tempfile
|
||||
from urllib.parse import urlparse
|
||||
|
||||
import click
|
||||
from git import exc as git_exc
|
||||
from git import Git
|
||||
from git import Repo
|
||||
|
@ -61,7 +60,6 @@ def git_handler(repo_url,
|
|||
path to ``repo_url``.
|
||||
:raises ValueError: If ``repo_url`` isn't a valid URL or doesn't begin
|
||||
with a valid protocol (http, https or ssh) for cloning.
|
||||
:raises NotADirectoryError: If ``repo_url`` isn't a valid directory path.
|
||||
|
||||
"""
|
||||
|
||||
|
@ -91,13 +89,9 @@ def git_handler(repo_url,
|
|||
else:
|
||||
LOG.debug('Treating repo_url=%s as an already-cloned repository. '
|
||||
'Attempting to checkout ref=%s', repo_url, ref)
|
||||
try:
|
||||
# get absolute path of what is probably a directory
|
||||
repo_url, _ = normalize_repo_path(repo_url)
|
||||
except Exception:
|
||||
msg = "The repo_url=%s is not a valid Git repo" % repo_url
|
||||
LOG.error(msg)
|
||||
raise NotADirectoryError(msg)
|
||||
|
||||
# Normalize the repo path.
|
||||
repo_url, _ = normalize_repo_path(repo_url)
|
||||
|
||||
repo = Repo(repo_url, search_parent_directories=True)
|
||||
if repo.is_dirty(untracked_files=True):
|
||||
|
@ -434,7 +428,8 @@ def normalize_repo_path(repo_url_or_path):
|
|||
:param repo_url_or_path: URL of remote Git repo or path to local Git repo.
|
||||
:returns: Tuple of root Git path or URL, additional subpath included (e.g.
|
||||
"deployment_files")
|
||||
:rtype: tuple
|
||||
:rtype: tuple[str, str]
|
||||
:raises GitInvalidRepoException: If the repo is invalid.
|
||||
|
||||
"""
|
||||
|
||||
|
@ -459,8 +454,8 @@ def normalize_repo_path(repo_url_or_path):
|
|||
repo_url_or_path = os.path.abspath(repo_url_or_path)
|
||||
|
||||
if not repo_url_or_path or not is_repository(repo_url_or_path):
|
||||
raise click.ClickException(
|
||||
"Specified site repo path=%s exists but is not a valid Git "
|
||||
"repository" % orig_repo_path)
|
||||
msg = "The repo_path=%s is not a valid Git repo" % (orig_repo_path)
|
||||
LOG.error(msg)
|
||||
raise exceptions.GitInvalidRepoException(repo_path=repo_url_or_path)
|
||||
|
||||
return repo_url_or_path, sub_path
|
||||
|
|
|
@ -21,6 +21,7 @@ import click
|
|||
import pytest
|
||||
|
||||
from pegleg import config
|
||||
from pegleg.engine import exceptions
|
||||
from pegleg.engine import repository
|
||||
from pegleg.engine import util
|
||||
|
||||
|
@ -238,10 +239,10 @@ def test_process_repositories_with_local_site_path_exists_not_repo(*_):
|
|||
"""Validate that when the site repo already exists but isn't a repository
|
||||
that an error is raised.
|
||||
"""
|
||||
with pytest.raises(click.ClickException) as exc:
|
||||
with pytest.raises(exceptions.GitInvalidRepoException) as exc:
|
||||
_test_process_repositories_inner(
|
||||
expected_extra_repos=TEST_REPOSITORIES)
|
||||
assert "is not a valid Git repository" in str(exc.value)
|
||||
assert "The repository path or URL is invalid" in str(exc.value)
|
||||
|
||||
|
||||
def test_process_repositories_with_repo_username():
|
||||
|
|
|
@ -400,9 +400,11 @@ def test_git_clone_invalid_url_type_raises_value_error():
|
|||
@pytest.mark.skipif(
|
||||
not test_utils.is_connected(),
|
||||
reason='git clone requires network connectivity.')
|
||||
def test_git_clone_invalid_local_repo_url_raises_notadirectory_error():
|
||||
url = False
|
||||
with pytest.raises(NotADirectoryError):
|
||||
@mock.patch.object(git, 'is_repository', autospec=True, return_value=False)
|
||||
@mock.patch('os.path.exists', return_value=True, autospec=True)
|
||||
def test_git_clone_invalid_local_repo_url_raises_invalid_repo_exc(*args):
|
||||
url = 'blah'
|
||||
with pytest.raises(exceptions.GitInvalidRepoException):
|
||||
git.git_handler(url, ref='master')
|
||||
|
||||
|
||||
|
|
Loading…
Reference in New Issue