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

@@ -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=
+    help='The path where the repo will be cloned. By default the repo will be '
-    '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,
+def site(*, site_repository, clone_path, extra_repositories, repo_key,
-         repo_key, repo_username):
+         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,
+def type(*, site_repository, clone_path, extra_repositories, repo_key,
-         repo_key, repo_username):
+         repo_username):
     """Group for repo-level actions, which include:
 
     * list: list all types across the repository

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,
+        return util.git.git_handler(
-                                    *args, **kwargs)
+            repo_url_or_path, clone_path=clone_path, *args, **kwargs)
     except exceptions.GitException as e:
         raise click.ClickException(e)
     except Exception as e:

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,
+def git_handler(repo_url,
-                auth_key=None, clone_path=None):
+                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,
+            return _try_git_clone(repo_url, ref, proxy_server, auth_key,
-                                  auth_key, clone_path)
+                                  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,
+def _try_git_clone(repo_url,
-                   auth_key=None, clone_path=None):
+                   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:

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:

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,
+            self, temp_clone_path):
-        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,
+        site_list = self.runner.invoke(
-                                                  '-r', repo_url, 'list'])
+            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,
+        assert os.path.exists(os.path.join(temp_clone_path, self.repo_name))
-            self.repo_name))
+        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))
 
-    def test_list_sites_using_local_repo_and_clone_path_option(self,
+    def test_list_sites_using_local_repo_and_clone_path_option(
-        temp_clone_path):
+            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,
+        site_list = self.runner.invoke(
-                                                  '-r', repo_path, 'list'])
+            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,
+    def test_list_sites_using_remote_repo_and_reuse_clone_path_option(
-        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 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,
+        site_list = self.runner.invoke(
-                                                  '-r', repo_url, 'list'])
+            cli.site, ['-p', temp_clone_path, '-r', repo_url, 'list'])
 
-        assert git.is_repository(os.path.join(temp_clone_path,
+        assert git.is_repository(os.path.join(temp_clone_path, self.repo_name))
-            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,
+        site_list = self.runner.invoke(
-                                                  '-r', repo_url, 'list'])
+            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 " \