docs: Add docstring information for pegleg.config

Pegleg's config module is used for basically keeping track
of CLI context information within global memory namespace.
None of the functions therein are properly documented which
can lead to confusion. This patch set adds clarifying
documentation for all the functions.

Change-Id: I93545331ffc3a83b593f654ee90fb6af3d067402
This commit is contained in:
Felipe Monteiro 2018-10-19 18:39:11 +01:00
parent e3d37db45e
commit 3b419c3bc4
7 changed files with 88 additions and 52 deletions

View File

@ -69,8 +69,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 '
@ -188,8 +187,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
@ -201,7 +200,7 @@ def site(*, site_repository, clone_path, extra_repositories,
config.set_site_repo(site_repository)
config.set_clone_path(clone_path)
config.set_extra_repo_store(extra_repositories or [])
config.set_extra_repo_overrides(extra_repositories or [])
config.set_repo_key(repo_key)
config.set_repo_username(repo_username)
@ -324,8 +323,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
@ -333,7 +332,7 @@ def type(*, site_repository, clone_path, extra_repositories,
"""
config.set_site_repo(site_repository)
config.set_clone_path(clone_path)
config.set_extra_repo_store(extra_repositories or [])
config.set_extra_repo_overrides(extra_repositories or [])
config.set_repo_key(repo_key)
config.set_repo_username(repo_username)

View File

@ -12,6 +12,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
# TODO(felipemonteiro): This pattern below should be swapped out for click
# context passing but will require a somewhat heavy code refactor. See:
# http://click.pocoo.org/5/commands/#nested-handling-and-contexts
try:
if GLOBAL_CONTEXT:
pass
@ -26,81 +30,109 @@ except NameError:
def get_site_repo():
"""Get the primary site repository specified via ``-r`` CLI flag."""
return GLOBAL_CONTEXT['site_repo']
def set_site_repo(r):
"""Set the primary site repository."""
GLOBAL_CONTEXT['site_repo'] = r.rstrip('/') + '/'
def get_clone_path():
"""Get specified clone path (corresponds to ``-p`` CLI flag)."""
return GLOBAL_CONTEXT['clone_path']
def set_clone_path(p):
"""Set specified clone path (corresponds to ``-p`` CLI flag)."""
GLOBAL_CONTEXT['clone_path'] = p
def get_extra_repo_store():
return GLOBAL_CONTEXT.get('extra_repo_store', [])
def get_extra_repo_overrides():
"""Get extra repository overrides specified via ``-e`` CLI flag."""
return GLOBAL_CONTEXT.get('extra_repo_overrides', [])
def set_extra_repo_store(r):
GLOBAL_CONTEXT['extra_repo_store'] = r
def set_extra_repo_overrides(r):
"""Set extra repository overrides.
.. note:: Only CLI should call this.
"""
GLOBAL_CONTEXT['extra_repo_overrides'] = r
def set_repo_key(k):
"""Set additional repository key, like extra metadata to track."""
GLOBAL_CONTEXT['repo_key'] = k
def get_repo_key():
"""Get additional repository key."""
return GLOBAL_CONTEXT.get('repo_key', None)
def set_repo_username(u):
"""Set repo username for SSH auth, corresponds to ``-u`` CLI flag."""
GLOBAL_CONTEXT['repo_username'] = u
def get_repo_username():
"""Get repo username for SSH auth."""
return GLOBAL_CONTEXT.get('repo_username', None)
def set_extra_repo_list(a):
"""Set the extra repository list to be used by ``pegleg.engine``."""
GLOBAL_CONTEXT['extra_repos'] = [r.rstrip('/') + '/' for r in a]
def add_extra_repo(a):
GLOBAL_CONTEXT['extra_repos'].append(a.rstrip('/') + '/')
def get_extra_repo_list():
"""Get the extra repository list.
.. note::
Use this instead of ``get_extra_repo_overrides`` as it handles
both overrides and site-definition.yaml defaults.
"""
return GLOBAL_CONTEXT['extra_repos']
def add_extra_repo(a):
"""Add an extra repo to the extra repository list."""
GLOBAL_CONTEXT['extra_repos'].append(a.rstrip('/') + '/')
def each_extra_repo():
"""Iterate over each extra repo."""
for a in GLOBAL_CONTEXT['extra_repos']:
yield a
def all_repos():
"""Return the primary site repo, in addition to all extra ones."""
repos = [get_site_repo()]
repos.extend(get_extra_repo_list())
return repos
def get_rel_site_path():
"""Get the relative site path name, default is "site"."""
return GLOBAL_CONTEXT.get('site_path', 'site')
def set_rel_site_path(p):
"""Set the relative site path name."""
p = p or 'site'
GLOBAL_CONTEXT['site_path'] = p
def get_rel_type_path():
"""Get the relative type path name, default is "type"."""
return GLOBAL_CONTEXT.get('type_path', 'type')
def set_rel_type_path(p):
"""Set the relative type path name."""
p = p or 'type'
GLOBAL_CONTEXT['type_path'] = p

View File

@ -234,7 +234,7 @@ def _process_repository_overrides(site_def_repos):
"""
# Extra repositories to process.
provided_repo_overrides = config.get_extra_repo_store()
provided_repo_overrides = config.get_extra_repo_overrides()
# Map repository names to the associated URL/revision for cloning.
repo_overrides = {}
@ -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:

View File

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

View File

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

View File

@ -183,7 +183,7 @@ def _test_process_repositories(site_repo=None,
elif repo_overrides:
with mock.patch.object(
config,
'get_extra_repo_store',
'get_extra_repo_overrides',
autospec=True,
return_value=list(repo_overrides.values())):
do_test()
@ -301,7 +301,7 @@ def test_process_repositiories_extraneous_user_repo_value(m_log, *_):
# Get rid of REPO_USERNAME through an override.
with mock.patch.object(
config,
'get_extra_repo_store',
'get_extra_repo_overrides',
autospec=True,
return_value=repo_overrides):
_test_process_repositories_inner(
@ -355,7 +355,7 @@ def test_process_repositiories_no_site_def_repos_with_extraneous_overrides(
# Provide repo overrides.
with mock.patch.object(
config,
'get_extra_repo_store',
'get_extra_repo_overrides',
autospec=True,
return_value=repo_overrides):
_test_process_repositories_inner(
@ -395,12 +395,12 @@ def test_process_repositories_without_repositories_key_in_site_definition(
autospec=True,
return_value=TEST_REPOSITORIES)
@mock.patch.object(util.git, 'is_repository', autospec=True, return_value=True)
@mock.patch.object(config, 'get_extra_repo_store', autospec=True)
@mock.patch.object(config, 'get_extra_repo_overrides', autospec=True)
def test_process_extra_repositories_malformed_format_raises_exception(
m_get_extra_repo_store, *_):
m_get_extra_repo_overrides, *_):
# Will fail since it doesn't contain "=".
broken_repo_url = 'broken_url'
m_get_extra_repo_store.return_value = [broken_repo_url]
m_get_extra_repo_overrides.return_value = [broken_repo_url]
error = ("The repository %s must be in the form of "
"name=repoUrl[@revision]" % broken_repo_url)

View File

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