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:
Felipe Monteiro 2018-10-10 23:55:32 -04:00
parent c498bfee81
commit 2e51779d57
5 changed files with 27 additions and 19 deletions

View File

@ -56,3 +56,8 @@ Git Exceptions
:members:
:show-inheritance:
:undoc-members:
.. autoexception:: pegleg.engine.exceptions.GitInvalidRepoException
:members:
:show-inheritance:
:undoc-members:

View File

@ -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")

View File

@ -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

View File

@ -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():

View File

@ -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')