From 0fcca0bcdb276afad8e893c42583fef510e67d37 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sat, 20 Oct 2018 00:45:50 -0400 Subject: [PATCH] refactor: Add convenient callback for processing site repos This patch set adds a callback that can be passed to @click.argument(site_name, callback=process_repositories_callback) This makes it easy to have Pegleg automatically clone done repositories for the specified site and check out all specified references for each repository. Change-Id: I7726a8aec1b1689bffbdbfe81204ed2a14ac4b87 --- pegleg/cli.py | 42 ++++++++++++++++++++++--------------- pegleg/engine/repository.py | 4 ++-- pegleg/engine/util/git.py | 19 +++++++++++------ tests/unit/conftest.py | 1 + tests/unit/test_cli.py | 40 +++++++++++++++++------------------ 5 files changed, 60 insertions(+), 46 deletions(-) diff --git a/pegleg/cli.py b/pegleg/cli.py index cfcc574e..cd15660c 100644 --- a/pegleg/cli.py +++ b/pegleg/cli.py @@ -29,6 +29,18 @@ CONTEXT_SETTINGS = { 'help_option_names': ['-h', '--help'], } + +def _process_repositories_callback(ctx, param, value): + """Convenient callback for ``@click.argument(site_name)``. + + Automatically processes repository information for the specified site. This + entails cloning all requires repositories and checking out specified + references for each repository. + """ + engine.repository.process_repositories(value) + return value + + MAIN_REPOSITORY_OPTION = click.option( '-r', '--site-repository', @@ -69,8 +81,7 @@ REPOSITORY_CLONE_PATH_OPTION = click.option( '-p', '--clone-path', 'clone_path', - help= - 'The path where the repo will be cloned. By default the repo will be ' + help='The path where the repo will be cloned. By default the repo will be ' 'cloned to the /tmp path. If this option is included and the repo already ' 'exists, then the repo will not be cloned again and the user must specify ' 'a new clone path or pass in the local copy of the repository as the site ' @@ -104,6 +115,9 @@ WARN_LINT_OPTION = click.option( multiple=True, help='Warn if linting check fails. -w takes priority over -x.') +SITE_REPOSITORY_ARGUMENT = click.argument( + 'site_name', callback=_process_repositories_callback) + @click.group(context_settings=CONTEXT_SETTINGS) @click.option( @@ -188,8 +202,8 @@ def lint_repo(*, fail_on_missing_sub_src, exclude_lint, warn_lint): @EXTRA_REPOSITORY_OPTION @REPOSITORY_USERNAME_OPTION @REPOSITORY_KEY_OPTION -def site(*, site_repository, clone_path, extra_repositories, - repo_key, repo_username): +def site(*, site_repository, clone_path, extra_repositories, repo_key, + repo_username): """Group for site-level actions, which include: * list: list available sites in a manifests repo @@ -235,7 +249,7 @@ def site(*, site_repository, clone_path, extra_repositories, 'warn_lint', multiple=True, help='Warn if linting check fails. -w takes priority over -x.') -@click.argument('site_name') +@SITE_REPOSITORY_ARGUMENT def collect(*, save_location, validate, exclude_lint, warn_lint, site_name): """Collects documents into a single site-definition.yaml file, which defines the entire site definition and contains all documents required @@ -247,9 +261,6 @@ def collect(*, save_location, validate, exclude_lint, warn_lint, site_name): Collect can lint documents prior to collection if the ``--validate`` flag is optionally included. """ - - engine.repository.process_repositories(site_name) - if validate: # Lint the primary repo prior to document collection. _lint_helper( @@ -268,7 +279,7 @@ def collect(*, save_location, validate, exclude_lint, warn_lint, site_name): type=click.File(mode='w'), default=sys.stdout, help='Where to output. Defaults to sys.stdout.') -def list_(*, output_stream): +def list_sites(*, output_stream): engine.repository.process_site_repository(update_config=True) engine.site.list_(output_stream) @@ -281,9 +292,8 @@ def list_(*, output_stream): type=click.File(mode='w'), default=sys.stdout, help='Where to output. Defaults to sys.stdout.') -@click.argument('site_name') +@SITE_REPOSITORY_ARGUMENT def show(*, output_stream, site_name): - engine.repository.process_repositories(site_name) engine.site.show(site_name, output_stream) @@ -295,9 +305,8 @@ def show(*, output_stream, site_name): type=click.File(mode='w'), default=sys.stdout, help='Where to output. Defaults to sys.stdout.') -@click.argument('site_name') +@SITE_REPOSITORY_ARGUMENT def render(*, output_stream, site_name): - engine.repository.process_repositories(site_name) engine.site.render(site_name, output_stream) @@ -305,12 +314,11 @@ def render(*, output_stream, site_name): @ALLOW_MISSING_SUBSTITUTIONS_OPTION @EXCLUDE_LINT_OPTION @WARN_LINT_OPTION -@click.argument('site_name') +@SITE_REPOSITORY_ARGUMENT def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name): """Lint a given site using checks defined in :mod:`pegleg.engine.errorcodes`. """ - engine.repository.process_repositories(site_name) _lint_helper( site_name=site_name, fail_on_missing_sub_src=fail_on_missing_sub_src, @@ -324,8 +332,8 @@ def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name): @EXTRA_REPOSITORY_OPTION @REPOSITORY_USERNAME_OPTION @REPOSITORY_KEY_OPTION -def type(*, site_repository, clone_path, extra_repositories, - repo_key, repo_username): +def type(*, site_repository, clone_path, extra_repositories, repo_key, + repo_username): """Group for repo-level actions, which include: * list: list all types across the repository diff --git a/pegleg/engine/repository.py b/pegleg/engine/repository.py index fcea6047..ab4e4158 100644 --- a/pegleg/engine/repository.py +++ b/pegleg/engine/repository.py @@ -312,8 +312,8 @@ def _handle_repository(repo_url_or_path, *args, **kwargs): clone_path = config.get_clone_path() try: - return util.git.git_handler(repo_url_or_path, clone_path=clone_path, - *args, **kwargs) + return util.git.git_handler( + repo_url_or_path, clone_path=clone_path, *args, **kwargs) except exceptions.GitException as e: raise click.ClickException(e) except Exception as e: diff --git a/pegleg/engine/util/git.py b/pegleg/engine/util/git.py index e638d7c7..426de237 100644 --- a/pegleg/engine/util/git.py +++ b/pegleg/engine/util/git.py @@ -29,8 +29,11 @@ LOG = logging.getLogger(__name__) __all__ = ('git_handler', ) -def git_handler(repo_url, ref=None, proxy_server=None, - auth_key=None, clone_path=None): +def git_handler(repo_url, + ref=None, + proxy_server=None, + auth_key=None, + clone_path=None): """Handle directories that are Git repositories. If ``repo_url`` is a valid URL for which a local repository doesn't @@ -75,8 +78,8 @@ def git_handler(repo_url, ref=None, proxy_server=None, # we need to clone the repo_url first since it doesn't exist and then # checkout the appropriate reference - and return the tmpdir if parsed_url.scheme in supported_clone_protocols: - return _try_git_clone(repo_url, ref, proxy_server, - auth_key, clone_path) + return _try_git_clone(repo_url, ref, proxy_server, auth_key, + clone_path) else: raise ValueError('repo_url=%s must use one of the following ' 'protocols: %s' % @@ -142,8 +145,11 @@ def _get_current_ref(repo_url): return None -def _try_git_clone(repo_url, ref=None, proxy_server=None, - auth_key=None, clone_path=None): +def _try_git_clone(repo_url, + ref=None, + proxy_server=None, + auth_key=None, + clone_path=None): """Try cloning Git repo from ``repo_url`` using the reference ``ref``. :param repo_url: URL of remote Git repo or path to local Git repo. @@ -168,6 +174,7 @@ def _try_git_clone(repo_url, ref=None, proxy_server=None, # and ensure we handle url/foo.git/ cases. prefix is 'tmp' by default. repo_name = repo_url.rstrip('/').split('/')[-1] temp_dir = os.path.join(clone_path, repo_name) + try: os.makedirs(temp_dir) except FileExistsError: diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index a05f82b4..fc0b171e 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -50,6 +50,7 @@ def clean_temporary_git_repos(): if os.path.isdir(path) and os.access(path, os.R_OK): if any(p.startswith('airship') for p in os.listdir(path)): yield path + try: yield finally: diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 2bc37fc6..ad4c34c8 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -26,6 +26,7 @@ from pegleg.engine.util import git from tests.unit import test_utils from tests.unit.fixtures import temp_clone_path + @pytest.mark.skipif( not test_utils.is_connected(), reason='git clone requires network connectivity.') @@ -59,8 +60,7 @@ class TestSiteCLIOptions(BaseCLIActionTest): ### clone_path tests ### def test_list_sites_using_remote_repo_and_clone_path_option( - self, - temp_clone_path): + self, temp_clone_path): """Validates clone_path (-p) option is working properly with site list action when using remote repo. Verify that the repo was cloned in the clone_path @@ -74,18 +74,16 @@ class TestSiteCLIOptions(BaseCLIActionTest): self.repo_rev) # Note that the -p option is used to specify the clone_folder - site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path, - '-r', repo_url, 'list']) + site_list = self.runner.invoke( + cli.site, ['-p', temp_clone_path, '-r', repo_url, 'list']) assert site_list.exit_code == 0 # Verify that the repo was cloned into the clone_path - assert os.path.exists(os.path.join(temp_clone_path, - self.repo_name)) - assert git.is_repository(os.path.join(temp_clone_path, - self.repo_name)) + assert os.path.exists(os.path.join(temp_clone_path, self.repo_name)) + assert git.is_repository(os.path.join(temp_clone_path, self.repo_name)) - def test_list_sites_using_local_repo_and_clone_path_option(self, - temp_clone_path): + def test_list_sites_using_local_repo_and_clone_path_option( + self, temp_clone_path): """Validates clone_path (-p) option is working properly with site list action when using a local repo. Verify that the clone_path has NO effect when using a local repo @@ -98,12 +96,13 @@ class TestSiteCLIOptions(BaseCLIActionTest): repo_path = self.treasuremap_path # Note that the -p option is used to specify the clone_folder - site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path, - '-r', repo_path, 'list']) + site_list = self.runner.invoke( + cli.site, ['-p', temp_clone_path, '-r', repo_path, 'list']) assert site_list.exit_code == 0 # Verify that passing in clone_path when using local repo has no effect - assert not os.path.exists(os.path.join(temp_clone_path, self.repo_name)) + assert not os.path.exists( + os.path.join(temp_clone_path, self.repo_name)) class TestSiteCLIOptionsNegative(BaseCLIActionTest): @@ -111,8 +110,8 @@ class TestSiteCLIOptionsNegative(BaseCLIActionTest): ### Negative clone_path tests ### - def test_list_sites_using_remote_repo_and_reuse_clone_path_option(self, - temp_clone_path): + def test_list_sites_using_remote_repo_and_reuse_clone_path_option( + self, temp_clone_path): """Validates clone_path (-p) option is working properly with site list action when using remote repo. Verify that the same repo can't be cloned in the same clone_path if it already exists @@ -126,16 +125,15 @@ class TestSiteCLIOptionsNegative(BaseCLIActionTest): self.repo_rev) # Note that the -p option is used to specify the clone_folder - site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path, - '-r', repo_url, 'list']) + site_list = self.runner.invoke( + cli.site, ['-p', temp_clone_path, '-r', repo_url, 'list']) - assert git.is_repository(os.path.join(temp_clone_path, - self.repo_name)) + assert git.is_repository(os.path.join(temp_clone_path, self.repo_name)) # Run site list for a second time to validate that the repo can't be # cloned twice in the same clone_path - site_list = self.runner.invoke(cli.site, ['-p', temp_clone_path, - '-r', repo_url, 'list']) + site_list = self.runner.invoke( + cli.site, ['-p', temp_clone_path, '-r', repo_url, 'list']) assert site_list.exit_code == 1 msg = "The repository already exists in the given path. Either " \