refactor: Allow site_by_params to take in list of fields
This patch set simplifies some code related to site_by_params by allowing it to take an iterable argument called *fields which specifies exactly which parameters to include from the site-definition.yaml. This means that no hard-coding is required to manually filter the params into the exact parameters required by follow-up function calls. This is done for better code maintenance. Change-Id: Ief6483dfbf3759204106330284e8e9b824b5567e
This commit is contained in:
parent
c20d714c61
commit
76f12648f6
|
@ -183,9 +183,7 @@ def _verify_no_unexpected_files(*, sitenames=None):
|
||||||
expected_directories = set()
|
expected_directories = set()
|
||||||
for site_name in sitenames:
|
for site_name in sitenames:
|
||||||
params = util.definition.load_as_params(site_name)
|
params = util.definition.load_as_params(site_name)
|
||||||
expected_directories.update(
|
expected_directories.update(util.files.directories_for(**params))
|
||||||
util.files.directories_for(
|
|
||||||
site_name=params['site_name'], site_type=params['site_type']))
|
|
||||||
LOG.debug('expected_directories: %s', expected_directories)
|
LOG.debug('expected_directories: %s', expected_directories)
|
||||||
found_directories = util.files.existing_directories()
|
found_directories = util.files.existing_directories()
|
||||||
LOG.debug('found_directories: %s', found_directories)
|
LOG.debug('found_directories: %s', found_directories)
|
||||||
|
|
|
@ -117,20 +117,12 @@ def list_(output_stream):
|
||||||
|
|
||||||
# Create a table to output site information for all sites for a given repo
|
# Create a table to output site information for all sites for a given repo
|
||||||
site_table = PrettyTable()
|
site_table = PrettyTable()
|
||||||
site_table.field_names = ['site_name', 'site_type']
|
field_names = ['site_name', 'site_type']
|
||||||
|
site_table.field_names = field_names
|
||||||
|
|
||||||
for site_name in util.files.list_sites():
|
for site_name in util.files.list_sites():
|
||||||
params = util.definition.load_as_params(site_name)
|
params = util.definition.load_as_params(site_name, *field_names)
|
||||||
# TODO(felipemonteiro): This is a temporary hack around legacy manifest
|
site_table.add_row(list(map(lambda k: params[k], field_names)))
|
||||||
# repositories containing the name of a directory that symbolizes a
|
|
||||||
# repository. Once all these manifest repositories migrate over to Git
|
|
||||||
# references instead, remove this hack.
|
|
||||||
# NOTE(felipemonteiro): The 'revision' information can instead be
|
|
||||||
# computed using :func:`process_site_repository` and storing into
|
|
||||||
# a configuration via a "set_site_revision" function, for example.
|
|
||||||
if 'revision' in params:
|
|
||||||
params.pop('revision')
|
|
||||||
site_table.add_row([params['site_name'], params['site_type']])
|
|
||||||
# Write table to specified output_stream
|
# Write table to specified output_stream
|
||||||
output_stream.write(site_table.get_string() + "\n")
|
output_stream.write(site_table.get_string() + "\n")
|
||||||
|
|
||||||
|
@ -141,6 +133,8 @@ def show(site_name, output_stream):
|
||||||
# Create a table to output site information for specific site
|
# Create a table to output site information for specific site
|
||||||
site_table = PrettyTable()
|
site_table = PrettyTable()
|
||||||
site_table.field_names = ['revision', 'site_name', 'site_type', 'files']
|
site_table.field_names = ['revision', 'site_name', 'site_type', 'files']
|
||||||
|
# TODO(felipemonteiro): Drop support for 'revision' once manifest
|
||||||
|
# repositories have removed it altogether.
|
||||||
if 'revision' in data.keys():
|
if 'revision' in data.keys():
|
||||||
for file in data['files']:
|
for file in data['files']:
|
||||||
site_table.add_row(
|
site_table.add_row(
|
||||||
|
|
|
@ -30,18 +30,25 @@ def load(site, primary_repo_base=None):
|
||||||
return files.slurp(path(site, primary_repo_base))
|
return files.slurp(path(site, primary_repo_base))
|
||||||
|
|
||||||
|
|
||||||
def load_as_params(site_name, primary_repo_base=None):
|
def load_as_params(site_name, *fields, primary_repo_base=None):
|
||||||
|
"""Load site definition for given ``site_name`` and return data as params.
|
||||||
|
|
||||||
|
:param str site_name: Name of the site.
|
||||||
|
:param iterable fields: List of parameter fields to return. Defaults to
|
||||||
|
``('site_name', 'site_type')``.
|
||||||
|
:param str primary_repo_base: Path to primary repository.
|
||||||
|
:returns: key-value pairs of parameters, whose keys are a subset of those
|
||||||
|
specified by ``fields``.
|
||||||
|
:rtype: dict
|
||||||
|
"""
|
||||||
|
if not fields:
|
||||||
|
# Default legacy fields.
|
||||||
|
fields = ('site_name', 'site_type')
|
||||||
|
|
||||||
definition = load(site_name, primary_repo_base)
|
definition = load(site_name, primary_repo_base)
|
||||||
# TODO(felipemonteiro): Currently we are filtering out "revision" from
|
|
||||||
# the params that are returned by this function because it is no longer
|
|
||||||
# supported. This is a workaround. As soon as the site definition repos
|
|
||||||
# switch to real repository format, then we can drop that workaround.
|
|
||||||
# Ideally, we should:
|
|
||||||
# 1) validate the site-definition.yaml format using lint module
|
|
||||||
# 2) extract only the required params here
|
|
||||||
params = definition.get('data', {})
|
params = definition.get('data', {})
|
||||||
params['site_name'] = site_name
|
params['site_name'] = site_name
|
||||||
return params
|
return {k: v for k, v in params.items() if k in fields}
|
||||||
|
|
||||||
|
|
||||||
def path(site_name, primary_repo_base=None):
|
def path(site_name, primary_repo_base=None):
|
||||||
|
@ -63,17 +70,14 @@ def pluck(site_definition, key):
|
||||||
|
|
||||||
def site_files(site_name):
|
def site_files(site_name):
|
||||||
params = load_as_params(site_name)
|
params = load_as_params(site_name)
|
||||||
for filename in files.search(
|
for filename in files.search(files.directories_for(**params)):
|
||||||
files.directories_for(
|
|
||||||
site_name=params['site_name'], site_type=params['site_type'])):
|
|
||||||
yield filename
|
yield filename
|
||||||
|
|
||||||
|
|
||||||
def site_files_by_repo(site_name):
|
def site_files_by_repo(site_name):
|
||||||
"""Yield tuples of repo_base, file_name."""
|
"""Yield tuples of repo_base, file_name."""
|
||||||
params = load_as_params(site_name)
|
params = load_as_params(site_name)
|
||||||
dir_map = files.directories_for_each_repo(
|
dir_map = files.directories_for_each_repo(**params)
|
||||||
site_name=params['site_name'], site_type=params['site_type'])
|
|
||||||
for repo, dl in dir_map.items():
|
for repo, dl in dir_map.items():
|
||||||
for filename in files.search(dl):
|
for filename in files.search(dl):
|
||||||
yield (repo, filename)
|
yield (repo, filename)
|
||||||
|
@ -93,8 +97,7 @@ def documents_for_each_site():
|
||||||
|
|
||||||
for sitename in sitenames:
|
for sitename in sitenames:
|
||||||
params = load_as_params(sitename)
|
params = load_as_params(sitename)
|
||||||
paths = files.directories_for(
|
paths = files.directories_for(**params)
|
||||||
site_name=params['site_name'], site_type=params['site_type'])
|
|
||||||
filenames = set(files.search(paths))
|
filenames = set(files.search(paths))
|
||||||
for filename in filenames:
|
for filename in filenames:
|
||||||
with open(filename) as f:
|
with open(filename) as f:
|
||||||
|
@ -116,8 +119,7 @@ def documents_for_site(sitename):
|
||||||
documents = []
|
documents = []
|
||||||
|
|
||||||
params = load_as_params(sitename)
|
params = load_as_params(sitename)
|
||||||
paths = files.directories_for(
|
paths = files.directories_for(**params)
|
||||||
site_name=params['site_name'], site_type=params['site_type'])
|
|
||||||
filenames = set(files.search(paths))
|
filenames = set(files.search(paths))
|
||||||
for filename in filenames:
|
for filename in filenames:
|
||||||
with open(filename) as f:
|
with open(filename) as f:
|
||||||
|
|
Loading…
Reference in New Issue