Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk): add noun aliasing to cli #7569

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions sdk/python/kfp/cli/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
sys.exit(1)
42 changes: 32 additions & 10 deletions sdk/python/kfp/cli/cli.py
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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(
Expand Down Expand Up @@ -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
51 changes: 51 additions & 0 deletions sdk/python/kfp/cli/cli_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# 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 sys
import unittest
from unittest import mock

from click import testing

# Docker is an optional install, but we need the import to succeed for tests,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, docker should be test dependency, maybe add it to requirements-test.txt? WDYT?

Copy link
Member Author

@connor-mccarthy connor-mccarthy Apr 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I did two things:

  1. I made the intent of the mock more clear with b5cee1c.
  2. I added docker (and mock, which had a TODO note) to the requirements-test.txt file with
    c1700b4. I chose not to pin the versions because we don't pin the versions in the other places these packages are/were listed (presubmit-tests-sdk.sh for mock and setup.py for docker.)

# so we patch it before importing from kfp.cli
try:
import docker
except ImportError:
sys.modules['docker'] = mock.Mock()

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)
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import docker # pylint: disable=unused-import
except ImportError:
sys.modules['docker'] = mock.Mock()
from kfp.cli import components
from kfp.cli import component


def _make_component(func_name: str,
Expand Down Expand Up @@ -69,8 +69,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
Expand Down Expand Up @@ -324,7 +324,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'],
Expand All @@ -339,7 +339,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',
Expand Down
13 changes: 13 additions & 0 deletions sdk/python/kfp/cli/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -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.
37 changes: 37 additions & 0 deletions sdk/python/kfp/cli/utils/aliased_plurals_group.py
Original file line number Diff line number Diff line change
@@ -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
55 changes: 55 additions & 0 deletions sdk/python/kfp/cli/utils/aliased_plurals_group_test.py
Original file line number Diff line number Diff line change
@@ -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()