diff --git a/sdk/python/kfp/cli/__main__.py b/sdk/python/kfp/cli/__main__.py index 17ad8a222cf..83ed0882a73 100644 --- a/sdk/python/kfp/cli/__main__.py +++ b/sdk/python/kfp/cli/__main__.py @@ -17,24 +17,12 @@ import click from kfp.cli import cli -from kfp.cli import components -from kfp.cli import diagnose_me_cli -from kfp.cli import experiment -from kfp.cli import pipeline -from kfp.cli import recurring_run -from kfp.cli import run def main(): logging.basicConfig(format='%(message)s', level=logging.INFO) - cli.cli.add_command(run.run) - cli.cli.add_command(recurring_run.recurring_run) - cli.cli.add_command(pipeline.pipeline) - cli.cli.add_command(diagnose_me_cli.diagnose_me) - cli.cli.add_command(experiment.experiment) - cli.cli.add_command(components.components) try: cli.cli(obj={}, auto_envvar_prefix='KFP') except Exception as e: click.echo(str(e), err=True) - sys.exit(1) \ No newline at end of file + sys.exit(1) diff --git a/sdk/python/kfp/cli/cli.py b/sdk/python/kfp/cli/cli.py index dd42ceba536..ba0d5ae5281 100644 --- a/sdk/python/kfp/cli/cli.py +++ b/sdk/python/kfp/cli/cli.py @@ -1,4 +1,4 @@ -# Copyright 2018 The Kubeflow Authors +# Copyright 2018-2022 The Kubeflow Authors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -12,12 +12,31 @@ # See the License for the specific language governing permissions and # limitations under the License. +from itertools import chain + import click +from kfp.cli import component +from kfp.cli import diagnose_me_cli +from kfp.cli import experiment +from kfp.cli import pipeline +from kfp.cli import recurring_run +from kfp.cli import run from kfp.cli.output import OutputFormat +from kfp.cli.utils import aliased_plurals_group from kfp.client import Client +COMMANDS = { + 'client': { + run.run, recurring_run.recurring_run, experiment.experiment, + pipeline.pipeline + }, + 'no_client': {diagnose_me_cli.diagnose_me, component.component} +} + -@click.group() +@click.group( + cls=aliased_plurals_group.AliasedPluralsGroup, + commands=list(chain.from_iterable(COMMANDS.values()))) # type: ignore @click.option('--endpoint', help='Endpoint of the KFP API service to connect.') @click.option('--iap-client-id', help='Client ID for IAP protected endpoint.') @click.option( @@ -46,11 +65,14 @@ def cli(ctx: click.Context, endpoint: str, iap_client_id: str, namespace: str, Feature stage: [Alpha](https://github.com/kubeflow/pipelines/blob/07328e5094ac2981d3059314cc848fbb71437a76/docs/release/feature-stages.md#alpha) """ - NO_CLIENT_COMMANDS = ['diagnose_me', 'components'] - if ctx.invoked_subcommand in NO_CLIENT_COMMANDS: - # Do not create a client for these subcommands - return - ctx.obj['client'] = Client(endpoint, iap_client_id, namespace, - other_client_id, other_client_secret) - ctx.obj['namespace'] = namespace - ctx.obj['output'] = output + client_commands = set( + chain.from_iterable([ + (command.name, f'{command.name}s') + for command in COMMANDS['client'] # type: ignore + ])) + + if ctx.invoked_subcommand in client_commands: + ctx.obj['client'] = Client(endpoint, iap_client_id, namespace, + other_client_id, other_client_secret) + ctx.obj['namespace'] = namespace + ctx.obj['output'] = output diff --git a/sdk/python/kfp/cli/cli_test.py b/sdk/python/kfp/cli/cli_test.py new file mode 100644 index 00000000000..91439d94f91 --- /dev/null +++ b/sdk/python/kfp/cli/cli_test.py @@ -0,0 +1,41 @@ +# Copyright 2022 The Kubeflow Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import functools +import unittest + +from click import testing +from kfp.cli import cli + + +class TestCli(unittest.TestCase): + + def setUp(self): + runner = testing.CliRunner() + self.invoke = functools.partial( + runner.invoke, cli=cli.cli, catch_exceptions=False, obj={}) + + def test_aliases_singular(self): + result = self.invoke(args=['component']) + self.assertEqual(result.exit_code, 0) + + def test_aliases_plural(self): + result = self.invoke(args=['components']) + self.assertEqual(result.exit_code, 0) + + def test_aliases_fails(self): + result = self.invoke(args=['componentss']) + self.assertEqual(result.exit_code, 2) + self.assertEqual("Error: Unrecognized command 'componentss'\n", + result.output) diff --git a/sdk/python/kfp/cli/components.py b/sdk/python/kfp/cli/component.py similarity index 96% rename from sdk/python/kfp/cli/components.py rename to sdk/python/kfp/cli/component.py index 31baea8fb43..7fb8774d8c2 100644 --- a/sdk/python/kfp/cli/components.py +++ b/sdk/python/kfp/cli/component.py @@ -323,45 +323,45 @@ def build_image(self, push_image: bool = True): @click.group() -def components(): +def component(): """Builds shareable, containerized components.""" pass -@components.command() +@component.command() @click.argument( - "components_directory", + 'components_directory', type=pathlib.Path, ) @click.option( - "--component-filepattern", + '--component-filepattern', type=str, default='**/*.py', - help="Filepattern to use when searching for KFP components. The" - " default searches all Python files in the specified directory.") + help='Filepattern to use when searching for KFP components. The' + ' default searches all Python files in the specified directory.') @click.option( - "--engine", + '--engine', type=str, - default="docker", + default='docker', help="Engine to use to build the component's container.") @click.option( - "--kfp-package-path", + '--kfp-package-path', type=pathlib.Path, default=None, - help="A pip-installable path to the KFP package.") + help='A pip-installable path to the KFP package.') @click.option( - "--overwrite-dockerfile", + '--overwrite-dockerfile', type=bool, is_flag=True, default=False, - help="Set this to true to always generate a Dockerfile" - " as part of the build process") + help='Set this to true to always generate a Dockerfile' + ' as part of the build process') @click.option( - "--push-image/--no-push-image", + '--push-image/--no-push-image', type=bool, is_flag=True, default=True, - help="Push the built image to its remote repository.") + help='Push the built image to its remote repository.') def build(components_directory: pathlib.Path, component_filepattern: str, engine: str, kfp_package_path: Optional[pathlib.Path], overwrite_dockerfile: bool, push_image: bool): diff --git a/sdk/python/kfp/cli/components_test.py b/sdk/python/kfp/cli/component_test.py similarity index 97% rename from sdk/python/kfp/cli/components_test.py rename to sdk/python/kfp/cli/component_test.py index ed90076079d..7ce2e28317e 100644 --- a/sdk/python/kfp/cli/components_test.py +++ b/sdk/python/kfp/cli/component_test.py @@ -14,21 +14,17 @@ """Tests for `components` command group in KFP CLI.""" import contextlib import pathlib -import sys import textwrap import unittest from typing import List, Optional, Union from unittest import mock +import docker from click import testing -# Docker is an optional install, but we need the import to succeed for tests. -# So we patch it before importing kfp.cli.components. -try: - import docker # pylint: disable=unused-import -except ImportError: - sys.modules['docker'] = mock.Mock() -from kfp.cli import components +docker = mock.Mock() + +from kfp.cli import component def _make_component(func_name: str, @@ -69,8 +65,8 @@ class Test(unittest.TestCase): def setUp(self) -> None: self.runner = testing.CliRunner() - self.cli = components.components - components._DOCKER_IS_PRESENT = True + self.cli = component.component + component._DOCKER_IS_PRESENT = True patcher = mock.patch('docker.from_env') self._docker_client = patcher.start().return_value @@ -324,7 +320,7 @@ def testKanikoEngineIsNotSupported(self): func_name='train', target_image='custom-image') _write_components('components.py', component) with self.assertWarnsRegex(DeprecationWarning, - r"The --engine option is deprecated"): + r'The --engine option is deprecated'): result = self.runner.invoke( self.cli, ['build', str(self._working_dir), '--engine=kaniko'], @@ -339,7 +335,7 @@ def testCloudBuildEngineIsNotSupported(self): _write_components('components.py', component) with self.assertWarnsRegex(DeprecationWarning, - r"The --engine option is deprecated"): + r'The --engine option is deprecated'): result = self.runner.invoke( self.cli, ['build', diff --git a/sdk/python/kfp/cli/utils/__init__.py b/sdk/python/kfp/cli/utils/__init__.py new file mode 100644 index 00000000000..94eae6abc61 --- /dev/null +++ b/sdk/python/kfp/cli/utils/__init__.py @@ -0,0 +1,13 @@ +# Copyright 2022 The Kubeflow Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/sdk/python/kfp/cli/utils/aliased_plurals_group.py b/sdk/python/kfp/cli/utils/aliased_plurals_group.py new file mode 100644 index 00000000000..79631827de7 --- /dev/null +++ b/sdk/python/kfp/cli/utils/aliased_plurals_group.py @@ -0,0 +1,37 @@ +# Copyright 2022 The Kubeflow Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from typing import List, Tuple, Union + +import click + + +class AliasedPluralsGroup(click.Group): + + def get_command(self, ctx: click.Context, cmd_name: str) -> click.Command: + regular = click.Group.get_command(self, ctx, cmd_name) + if regular is not None: + return regular + elif cmd_name.endswith('s'): + singular = click.Group.get_command(self, ctx, cmd_name[:-1]) + if singular is not None: + return singular + raise click.UsageError(f"Unrecognized command '{cmd_name}'") + + def resolve_command( + self, ctx: click.Context, args: List[str] + ) -> Tuple[Union[str, None], Union[click.Command, None], List[str]]: + # always return the full command name + _, cmd, args = super().resolve_command(ctx, args) + return cmd.name, cmd, args # type: ignore diff --git a/sdk/python/kfp/cli/utils/aliased_plurals_group_test.py b/sdk/python/kfp/cli/utils/aliased_plurals_group_test.py new file mode 100644 index 00000000000..45cc054580a --- /dev/null +++ b/sdk/python/kfp/cli/utils/aliased_plurals_group_test.py @@ -0,0 +1,55 @@ +# Copyright 2022 The Kubeflow Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +import click +from click import testing +from kfp.cli.utils import aliased_plurals_group + + +@click.group(cls=aliased_plurals_group.AliasedPluralsGroup) +def cli(): + pass + + +@cli.command() +def command(): + click.echo('Called command.') + + +class TestAliasedPluralsGroup(unittest.TestCase): + + def setUp(self): + self.runner = testing.CliRunner() + + def test_aliases_default_success(self): + result = self.runner.invoke(cli, ['command']) + self.assertEqual(result.exit_code, 0) + self.assertEqual(result.output, 'Called command.\n') + + def test_aliases_plural_success(self): + result = self.runner.invoke(cli, ['commands']) + self.assertEqual(result.exit_code, 0) + self.assertEqual(result.output, 'Called command.\n') + + def test_aliases_failure(self): + result = self.runner.invoke(cli, ['commandss']) + self.assertEqual(result.exit_code, 2) + self.assertEqual("Error: Unrecognized command 'commandss'\n", + result.output) + + +if __name__ == '__main__': + unittest.main() diff --git a/sdk/python/requirements-test.txt b/sdk/python/requirements-test.txt index bc04b4960a7..5d4a1e3c1ac 100644 --- a/sdk/python/requirements-test.txt +++ b/sdk/python/requirements-test.txt @@ -1 +1,3 @@ -r requirements.txt +docker +mock diff --git a/test/presubmit-tests-sdk.sh b/test/presubmit-tests-sdk.sh index d0f0f658690..9df15e02fbd 100755 --- a/test/presubmit-tests-sdk.sh +++ b/test/presubmit-tests-sdk.sh @@ -17,7 +17,6 @@ source_root=$(pwd) python3 -m pip install --upgrade pip python3 -m pip install coverage==4.5.4 coveralls==1.9.2 -python3 -m pip install mock # TODO: Remove when added to requirements python3 -m pip install -r "$source_root/sdk/python/requirements-test.txt" python3 -m pip install --upgrade protobuf