From 40c12b417e1e007c0c969dd6ed655fee9e28ef0d Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Fri, 14 Sep 2018 08:08:21 -0600 Subject: [PATCH] Remove unused `impacted` CLI command This patch set removes the impacted CLI command for the following reasons: 1) it currently isn't being used by anyone or any automation 2) it's not clear how to use it 3) making it work with git diff or git show in order to determine changed files automatically is sleaker design but currently there is no use case for this command Associated engine code, CLI documentation and helper functionality only associated with this command are dropped. Change-Id: Ic09951e988b82a386d2a1a7b9ce9d77f78bd00d7 --- doc/source/cli.rst | 17 ----------- src/bin/pegleg/pegleg/cli.py | 18 ------------ src/bin/pegleg/pegleg/engine/site.py | 28 +------------------ .../pegleg/pegleg/engine/util/definition.py | 7 +++++ .../unit/engine/test_selectable_linting.py | 1 + 5 files changed, 9 insertions(+), 62 deletions(-) diff --git a/doc/source/cli.rst b/doc/source/cli.rst index dc05741d..32174895 100644 --- a/doc/source/cli.rst +++ b/doc/source/cli.rst @@ -168,23 +168,6 @@ Example with validation: -e global=/opt/aic-clcp-manifests \ collect -s /workspace -x P004 --validate -Impacted --------- - -Find sites impacted by changed files. - -**-i / --input** - -List of impacted files. - -**-o / --output** - -Where to output. - -:: - - ./pegleg impacted -i -o - List ---- diff --git a/src/bin/pegleg/pegleg/cli.py b/src/bin/pegleg/pegleg/cli.py index 533b8249..3a14895e 100644 --- a/src/bin/pegleg/pegleg/cli.py +++ b/src/bin/pegleg/pegleg/cli.py @@ -137,24 +137,6 @@ def collect(*, save_location, validate, exclude_lint, warn_lint, site_name): engine.site.collect(site_name, save_location) -@site.command(help='Find sites impacted by changed files') -@click.option( - '-i', - '--input', - 'input_stream', - type=click.File(mode='r'), - default=sys.stdin, - help='List of impacted files') -@click.option( - '-o', - '--output', - 'output_stream', - type=click.File(mode='w'), - default=sys.stdout) -def impacted(*, input_stream, output_stream): - engine.site.impacted(input_stream, output_stream) - - @site.command('list', help='List known sites') @click.option( '-o', diff --git a/src/bin/pegleg/pegleg/engine/site.py b/src/bin/pegleg/pegleg/engine/site.py index e19ceec7..b78accf3 100644 --- a/src/bin/pegleg/pegleg/engine/site.py +++ b/src/bin/pegleg/pegleg/engine/site.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import collections import csv import json import logging @@ -23,7 +22,7 @@ import yaml from pegleg.engine import util -__all__ = ('collect', 'impacted', 'list_', 'show', 'render') +__all__ = ('collect', 'list_', 'show', 'render') LOG = logging.getLogger(__name__) @@ -89,20 +88,6 @@ def collect(site_name, save_location): _collect_to_stdout(site_name) -def impacted(input_stream, output_stream): - mapping = _build_impact_mapping() - impacted_sites = set() - - for line in input_stream: - line = line.strip() - directory = util.files.directory_for(path=line) - if directory is not None: - impacted_sites.update(mapping[directory]) - - for site_name in sorted(impacted_sites): - output_stream.write(site_name + '\n') - - def render(site_name, output_stream): documents = [] for filename in util.definition.site_files(site_name): @@ -148,14 +133,3 @@ def show(site_name, output_stream): data = util.definition.load_as_params(site_name) data['files'] = list(util.definition.site_files(site_name)) json.dump(data, output_stream, indent=2, sort_keys=True) - - -def _build_impact_mapping(): - mapping = collections.defaultdict(set) - - for site_name in util.files.list_sites(): - params = util.definition.load_as_params(site_name) - for directory in util.files.directories_for(**params): - mapping[directory].add(site_name) - - return mapping diff --git a/src/bin/pegleg/pegleg/engine/util/definition.py b/src/bin/pegleg/pegleg/engine/util/definition.py index 6db78233..32bd3c0c 100644 --- a/src/bin/pegleg/pegleg/engine/util/definition.py +++ b/src/bin/pegleg/pegleg/engine/util/definition.py @@ -56,6 +56,13 @@ def load(site, primary_repo_base=None): def load_as_params(site_name, primary_repo_base=None): 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['site_name'] = site_name return params diff --git a/src/bin/pegleg/tests/unit/engine/test_selectable_linting.py b/src/bin/pegleg/tests/unit/engine/test_selectable_linting.py index dcf374ee..27283696 100644 --- a/src/bin/pegleg/tests/unit/engine/test_selectable_linting.py +++ b/src/bin/pegleg/tests/unit/engine/test_selectable_linting.py @@ -25,6 +25,7 @@ no longer relevant and so the lint logic for this rule needs to be updated. For more information, see: https://storyboard.openstack.org/#!/story/2003762 """ + @mock.patch.object(lint, '_verify_deckhand_render', return_value=[]) @mock.patch.object(lint, '_verify_no_unexpected_files', return_value=[]) def test_lint_excludes_P001(*args):