From ec0081372a54e4fdf6d13d5c8c3388896dae8e71 Mon Sep 17 00:00:00 2001 From: Connor McCarthy Date: Fri, 29 Apr 2022 11:20:57 -0600 Subject: [PATCH] feat(sdk): improve cli help text (#7618) * add docstring parsing utils * implement docstring parsing for help text * implement smoke test of all cli commands * clean up use of optional in docstring * make command descriptions more consistent * fix disable_type_check * update release notes --- sdk/RELEASE.md | 2 + sdk/python/kfp/cli/cli.py | 23 ++-- sdk/python/kfp/cli/cli_test.py | 31 ++++++ sdk/python/kfp/cli/component.py | 1 + sdk/python/kfp/cli/diagnose_me_cli.py | 9 +- sdk/python/kfp/cli/dsl_compile.py | 26 +++-- sdk/python/kfp/cli/experiment.py | 42 ++++--- sdk/python/kfp/cli/pipeline.py | 91 +++++++++------ sdk/python/kfp/cli/recurring_run.py | 94 ++++++++++------ sdk/python/kfp/cli/run.py | 65 ++++++----- sdk/python/kfp/cli/utils/parsing.py | 69 ++++++++++++ sdk/python/kfp/cli/utils/parsing_test.py | 136 +++++++++++++++++++++++ sdk/python/kfp/client/client.py | 48 ++++---- 13 files changed, 475 insertions(+), 162 deletions(-) create mode 100644 sdk/python/kfp/cli/utils/parsing.py create mode 100644 sdk/python/kfp/cli/utils/parsing_test.py diff --git a/sdk/RELEASE.md b/sdk/RELEASE.md index b1124f5587a..b32a2ebb412 100644 --- a/sdk/RELEASE.md +++ b/sdk/RELEASE.md @@ -37,6 +37,8 @@ * Add pipeline_task_name to PipelineTaskFinalStatus [\#7464](https://github.com/kubeflow/pipelines/pull/7464) * Depends on `kfp-pipeline-spec>=0.1.14,<0.2.0` [\#7464](https://github.com/kubeflow/pipelines/pull/7464) * Depends on `google-cloud-storage>=2.2.1,<3` [\#7493](https://github.com/kubeflow/pipelines/pull/7493) +* Add additional methods to `kfp.client.Client` [\#7562](https://github.com/kubeflow/pipelines/pull/7562), [\#7463](https://github.com/kubeflow/pipelines/pull/7463) +* Migrate V1 CLI to V2, with improvements [\#7547](https://github.com/kubeflow/pipelines/pull/7547), [\#7558](https://github.com/kubeflow/pipelines/pull/7558), [\#7559](https://github.com/kubeflow/pipelines/pull/7559), [\#7560](https://github.com/kubeflow/pipelines/pull/7560), , [\#7569](https://github.com/kubeflow/pipelines/pull/7569), [\#7567](https://github.com/kubeflow/pipelines/pull/7567), [\#7603](https://github.com/kubeflow/pipelines/pull/7603), [\#7606](https://github.com/kubeflow/pipelines/pull/7606), [\#7607](https://github.com/kubeflow/pipelines/pull/7607), [\#7628](https://github.com/kubeflow/pipelines/pull/7628), [\#7618](https://github.com/kubeflow/pipelines/pull/7618) ## Documentation Updates diff --git a/sdk/python/kfp/cli/cli.py b/sdk/python/kfp/cli/cli.py index 10f6af0f1f2..85dd19c360f 100644 --- a/sdk/python/kfp/cli/cli.py +++ b/sdk/python/kfp/cli/cli.py @@ -17,6 +17,7 @@ import click import kfp +from kfp import client from kfp.cli import component from kfp.cli import diagnose_me_cli from kfp.cli import dsl @@ -26,7 +27,7 @@ from kfp.cli import run from kfp.cli.output import OutputFormat from kfp.cli.utils import aliased_plurals_group -from kfp.client import Client +from kfp.cli.utils import parsing COMMANDS = { 'client': { @@ -69,20 +70,21 @@ def _install_completion(shell: str) -> None: '--install-completion', type=click.Choice(list(SHELL_FILES.keys())), default=None) -@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('--endpoint', help=parsing.get_param_descr(client.Client, 'host')) +@click.option( + '--iap-client-id', help=parsing.get_param_descr(client.Client, 'client_id')) @click.option( '-n', '--namespace', default='kubeflow', show_default=True, - help='Kubernetes namespace to connect to the KFP API.') + help=parsing.get_param_descr(client.Client, 'namespace')) @click.option( '--other-client-id', - help='Client ID for IAP protected endpoint to obtain the refresh token.') + help=parsing.get_param_descr(client.Client, 'other_client_id')) @click.option( '--other-client-secret', - help='Client ID for IAP protected endpoint to obtain the refresh token.') + help=parsing.get_param_descr(client.Client, 'other_client_secret')) @click.option( '--output', type=click.Choice(list(map(lambda x: x.name, OutputFormat))), @@ -94,10 +96,9 @@ def _install_completion(shell: str) -> None: def cli(ctx: click.Context, endpoint: str, iap_client_id: str, namespace: str, other_client_id: str, other_client_secret: str, output: OutputFormat, show_completion: str, install_completion: str): - """kfp is the command line interface to KFP service. + """Kubeflow Pipelines CLI. - Feature stage: - [Alpha](https://github.com/kubeflow/pipelines/blob/07328e5094ac2981d3059314cc848fbb71437a76/docs/release/feature-stages.md#alpha) + The KFP CLI is currently in Alpha feature stage (https://github.com/kubeflow/pipelines/blob/07328e5094ac2981d3059314cc848fbb71437a76/docs/release/feature-stages.md#alpha). """ if show_completion: click.echo(_create_completion(show_completion)) @@ -114,7 +115,7 @@ def cli(ctx: click.Context, endpoint: str, iap_client_id: str, namespace: str, if ctx.invoked_subcommand not in 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['client'] = 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 index 80b419a5ec6..2c83d22a055 100644 --- a/sdk/python/kfp/cli/cli_test.py +++ b/sdk/python/kfp/cli/cli_test.py @@ -22,6 +22,7 @@ from typing import Any, Dict, List, Optional from unittest import mock +import click import yaml from absl.testing import parameterized from click import testing @@ -256,5 +257,35 @@ def test_deprecated_command_is_found(self): self.assertEqual(result.exit_code, 0) +info_dict = cli.cli.to_info_dict(ctx=click.Context(cli.cli)) +commands_dict = { + command: list(body.get('commands', {}).keys()) + for command, body in info_dict['commands'].items() +} +noun_verb_list = [ + (noun, verb) for noun, verbs in commands_dict.items() for verb in verbs +] + + +class TestSmokeTestAllCommandsWithHelp(parameterized.TestCase): + + @classmethod + def setUpClass(cls): + cls.runner = testing.CliRunner() + + cls.vals = [('run', 'list')] + + @parameterized.parameters(*noun_verb_list) + def test(self, noun: str, verb: str): + with mock.patch('kfp.cli.cli.client.Client'): + result = self.runner.invoke( + args=[noun, verb, '--help'], + cli=cli.cli, + catch_exceptions=False, + obj={}) + self.assertTrue(result.output.startswith('Usage: ')) + self.assertEqual(result.exit_code, 0) + + if __name__ == '__main__': unittest.main() diff --git a/sdk/python/kfp/cli/component.py b/sdk/python/kfp/cli/component.py index 7fb8774d8c2..f2360f7e1b1 100644 --- a/sdk/python/kfp/cli/component.py +++ b/sdk/python/kfp/cli/component.py @@ -366,6 +366,7 @@ def build(components_directory: pathlib.Path, component_filepattern: str, engine: str, kfp_package_path: Optional[pathlib.Path], overwrite_dockerfile: bool, push_image: bool): """Builds containers for KFP v2 Python-based components.""" + if engine != 'docker': warnings.warn( 'The --engine option is deprecated and does not need to be passed. Only Docker engine is supported and will be used by default.', diff --git a/sdk/python/kfp/cli/diagnose_me_cli.py b/sdk/python/kfp/cli/diagnose_me_cli.py index 8a3564b0b33..b03f9cb7643 100644 --- a/sdk/python/kfp/cli/diagnose_me_cli.py +++ b/sdk/python/kfp/cli/diagnose_me_cli.py @@ -5,7 +5,8 @@ from typing import Dict, List, Text, Union import click -from kfp.cli.diagnose_me import dev_env, gcp +from kfp.cli.diagnose_me import dev_env +from kfp.cli.diagnose_me import gcp from kfp.cli.diagnose_me import kubernetes_cluster from kfp.cli.diagnose_me import kubernetes_cluster as k8 from kfp.cli.diagnose_me import utility @@ -34,11 +35,7 @@ @click.pass_context def diagnose_me(ctx: click.Context, json: bool, project_id: str, namespace: str): - """Runs KFP environment diagnostic with specified parameters. - - Feature stage: - [Alpha](https://github.com/kubeflow/pipelines/blob/07328e5094ac2981d3059314cc848fbb71437a76/docs/release/feature-stages.md#alpha) - """ + """Runs KFP environment diagnostic.""" # validate kubectl, gcloud , and gsutil exist local_env_gcloud_sdk = gcp.get_gcp_configuration( gcp.Commands.GET_GCLOUD_VERSION, diff --git a/sdk/python/kfp/cli/dsl_compile.py b/sdk/python/kfp/cli/dsl_compile.py index 4f6b55bc30a..4a0b22938ba 100644 --- a/sdk/python/kfp/cli/dsl_compile.py +++ b/sdk/python/kfp/cli/dsl_compile.py @@ -20,6 +20,7 @@ import click from kfp import compiler +from kfp.cli.utils import parsing from kfp.components import pipeline_context @@ -28,17 +29,17 @@ def _compile_pipeline_function( function_name: Optional[str], pipeline_parameters: Optional[Mapping[str, Any]], package_path: str, - type_check: bool, + disable_type_check: bool, ) -> None: """Compiles a pipeline function. Args: pipeline_funcs: A list of pipeline_functions. - function_name: The name of the pipeline function to compile if there were + function_name: The name of the pipeline function to compile if there are multiple. - pipeline_parameters: The pipeline parameters as a dict of {name: value}. - package_path: The output path of the compiled result. - type_check: Whether to enable the type checking. + pipeline_parameters: The pipeline parameters as a dictionary. + package_path: Path to write the compiled result. + disable_type_check: Whether to disable type checking. """ if len(pipeline_funcs) == 0: raise ValueError( @@ -65,7 +66,7 @@ def _compile_pipeline_function( pipeline_func=pipeline_func, pipeline_parameters=pipeline_parameters, package_path=package_path, - type_check=type_check) + type_check=not disable_type_check) class PipelineCollectorContext(): @@ -93,13 +94,13 @@ def __exit__(self, *args): '--output', type=str, required=True, - help='Local path to the output PipelineJob JSON file.') + help=parsing.get_param_descr(_compile_pipeline_function, 'package_path')) @click.option( '--function', 'function_name', type=str, default=None, - help='The name of the function to compile if there are multiple.') + help=parsing.get_param_descr(_compile_pipeline_function, 'function_name')) @click.option( '--pipeline-parameters', type=str, @@ -109,13 +110,14 @@ def __exit__(self, *args): '--disable-type-check', is_flag=True, default=False, - help='Disable the type check. Default: type check is not disabled.') + help=parsing.get_param_descr(_compile_pipeline_function, + 'disable_type_check')) def dsl_compile( py: str, output: str, function_name: Optional[str] = None, pipeline_parameters: str = None, - disable_type_check: bool = True, + disable_type_check: bool = False, ) -> None: """Compiles a pipeline written in a .py file.""" sys.path.insert(0, os.path.dirname(py)) @@ -128,7 +130,7 @@ def dsl_compile( pipeline_parameters) if pipeline_parameters is not None else {} except json.JSONDecodeError as e: logging.error( - f"Failed to parse --pipeline-parameters argument: {pipeline_parameters}" + f'Failed to parse --pipeline-parameters argument: {pipeline_parameters}' ) raise e _compile_pipeline_function( @@ -136,7 +138,7 @@ def dsl_compile( function_name=function_name, pipeline_parameters=parsed_parameters, package_path=output, - type_check=not disable_type_check, + disable_type_check=disable_type_check, ) finally: del sys.path[0] diff --git a/sdk/python/kfp/cli/experiment.py b/sdk/python/kfp/cli/experiment.py index c9b0de2b661..09abbca29da 100644 --- a/sdk/python/kfp/cli/experiment.py +++ b/sdk/python/kfp/cli/experiment.py @@ -3,8 +3,10 @@ import click import kfp_server_api +from kfp import client from kfp.cli.output import OutputFormat from kfp.cli.output import print_output +from kfp.cli.utils import parsing from kfp_server_api.models.api_experiment import ApiExperiment @@ -15,7 +17,11 @@ def experiment(): @experiment.command() -@click.option('-d', '--description', help='Description of the experiment.') +@click.option( + '-d', + '--description', + help=parsing.get_param_descr(client.Client.create_experiment, + 'description')) @click.argument('name') @click.pass_context def create(ctx: click.Context, description: str, name: str): @@ -30,20 +36,21 @@ def create(ctx: click.Context, description: str, name: str): @experiment.command() @click.option( - '--page-token', default='', help='Token for starting of the page.') + '--page-token', + default='', + help=parsing.get_param_descr(client.Client.list_experiments, 'page_token')) @click.option( - '-m', '--max-size', default=100, help='Max size of the listed experiments.') + '-m', + '--max-size', + default=100, + help=parsing.get_param_descr(client.Client.list_experiments, 'page_size')) @click.option( '--sort-by', default='created_at desc', - help="Can be '[field_name]', '[field_name] desc'. For example, 'name desc'." -) + help=parsing.get_param_descr(client.Client.list_experiments, 'sort_by')) @click.option( '--filter', - help=( - 'filter: A url-encoded, JSON-serialized Filter protocol buffer ' - '(see [filter.proto](https://github.com/kubeflow/pipelines/blob/master/backend/api/filter.proto)).' - )) + help=parsing.get_param_descr(client.Client.list_experiments, 'filter')) @click.pass_context def list(ctx: click.Context, page_token: str, max_size: int, sort_by: str, filter: str): @@ -70,7 +77,7 @@ def list(ctx: click.Context, page_token: str, max_size: int, sort_by: str, @click.argument('experiment-id') @click.pass_context def get(ctx: click.Context, experiment_id: str): - """Get detailed information about an experiment.""" + """Get information about an experiment.""" client = ctx.obj['client'] output_format = ctx.obj['output'] @@ -120,17 +127,20 @@ def _display_experiment(exp: kfp_server_api.ApiExperiment, print_output(dict(table), [], output_format) +either_option_required = 'Either --experiment-id or --experiment-name is required.' + + @experiment.command() @click.option( '--experiment-id', default=None, - help='The ID of the experiment to archive, can only supply either an experiment ID or name.' + help=parsing.get_param_descr(client.Client.archive_experiment, + 'experiment_id') + ' ' + either_option_required ) @click.option( '--experiment-name', default=None, - help='The name of the experiment to archive, can only supply either an experiment ID or name.' -) + help='Name of the experiment.' + ' ' + either_option_required) @click.pass_context def archive(ctx: click.Context, experiment_id: str, experiment_name: str): """Archive an experiment.""" @@ -152,13 +162,13 @@ def archive(ctx: click.Context, experiment_id: str, experiment_name: str): @click.option( '--experiment-id', default=None, - help='The ID of the experiment to archive, can only supply either an experiment ID or name.' + help=parsing.get_param_descr(client.Client.unarchive_experiment, + 'experiment_id') + ' ' + either_option_required ) @click.option( '--experiment-name', default=None, - help='The name of the experiment to archive, can only supply either an experiment ID or name.' -) + help='Name of the experiment.' + ' ' + either_option_required) @click.pass_context def unarchive(ctx: click.Context, experiment_id: str, experiment_name: str): """Unarchive an experiment.""" diff --git a/sdk/python/kfp/cli/pipeline.py b/sdk/python/kfp/cli/pipeline.py index 76165eb44db..2da28430404 100644 --- a/sdk/python/kfp/cli/pipeline.py +++ b/sdk/python/kfp/cli/pipeline.py @@ -17,8 +17,10 @@ import click import kfp_server_api +from kfp import client from kfp.cli.output import OutputFormat from kfp.cli.output import print_output +from kfp.cli.utils import parsing @click.group() @@ -28,15 +30,22 @@ def pipeline(): @pipeline.command() -@click.option('-p', '--pipeline-name', help='Name of the pipeline.') -@click.option('-d', '--description', help='Description for the pipeline.') +@click.option( + '-p', + '--pipeline-name', + help=parsing.get_param_descr(client.Client.upload_pipeline, + 'pipeline_name')) +@click.option( + '-d', + '--description', + help=parsing.get_param_descr(client.Client.upload_pipeline, 'description')) @click.argument('package-file') @click.pass_context def upload(ctx: click.Context, pipeline_name: str, package_file: str, description: str = None): - """Upload a KFP pipeline.""" + """Upload a pipeline.""" client = ctx.obj['client'] output_format = ctx.obj['output'] if not pipeline_name: @@ -47,14 +56,30 @@ def upload(ctx: click.Context, click.echo('Pipeline uploaded successfully.') +either_option_required = 'Either --pipeline-id or --pipeline-name is required.' + + @pipeline.command() -@click.option('-p', '--pipeline-id', help='ID of the pipeline', required=False) -@click.option('-n', '--pipeline-name', help='Name of pipeline', required=False) +@click.option( + '-p', + '--pipeline-id', + required=False, + help=parsing.get_param_descr(client.Client.upload_pipeline_version, + 'pipeline_id') + ' ' + either_option_required) +@click.option( + '-n', + '--pipeline-name', + required=False, + help=parsing.get_param_descr(client.Client.upload_pipeline_version, + 'pipeline_name') + ' ' + either_option_required +) @click.option( '-v', '--pipeline-version', - help='Name of the pipeline version', - required=True) + help=parsing.get_param_descr(client.Client.upload_pipeline_version, + 'pipeline_version_name'), + required=True, +) @click.argument('package-file') @click.pass_context def upload_version(ctx: click.Context, @@ -62,16 +87,17 @@ def upload_version(ctx: click.Context, pipeline_version: str, pipeline_id: Optional[str] = None, pipeline_name: Optional[str] = None): - """Upload a version of the KFP pipeline.""" + """Upload a version of a pipeline.""" client = ctx.obj['client'] output_format = ctx.obj['output'] if bool(pipeline_id) == bool(pipeline_name): - raise ValueError("Need to supply 'pipeline-name' or 'pipeline-id'") + raise ValueError(either_option_required) if pipeline_name is not None: pipeline_id = client.get_pipeline_id(name=pipeline_name) if pipeline_id is None: raise ValueError( f"Can't find a pipeline with name: {pipeline_name}") + # TODO: this is broken version = client.pipeline_uploads.upload_pipeline_version( package_file, name=pipeline_version, pipelineid=pipeline_id) _display_pipeline_version(version, output_format) @@ -79,24 +105,25 @@ def upload_version(ctx: click.Context, @pipeline.command() @click.option( - '--page-token', default='', help='Token for starting of the page.') + '--page-token', + default='', + help=parsing.get_param_descr(client.Client.list_pipelines, 'page_token')) @click.option( - '-m', '--max-size', default=100, help='Max size of the listed pipelines.') + '-m', + '--max-size', + default=100, + help=parsing.get_param_descr(client.Client.list_pipelines, 'page_size')) @click.option( '--sort-by', default='created_at desc', - help="Can be '[field_name]', '[field_name] desc'. For example, 'name desc'." -) + help=parsing.get_param_descr(client.Client.list_pipelines, 'sort_by')) @click.option( '--filter', - help=( - 'filter: A url-encoded, JSON-serialized Filter protocol buffer ' - '(see [filter.proto](https://github.com/kubeflow/pipelines/blob/master/backend/api/filter.proto)).' - )) + help=parsing.get_param_descr(client.Client.list_pipelines, 'filter')) @click.pass_context def list(ctx: click.Context, page_token: str, max_size: int, sort_by: str, filter: str): - """List uploaded KFP pipelines.""" + """List pipelines.""" client = ctx.obj['client'] output_format = ctx.obj['output'] @@ -118,27 +145,29 @@ def list(ctx: click.Context, page_token: str, max_size: int, sort_by: str, @pipeline.command() @click.argument('pipeline-id') @click.option( - '--page-token', default='', help='Token for starting of the page.') + '--page-token', + default='', + help=parsing.get_param_descr(client.Client.list_pipeline_versions, + 'page_token')) @click.option( '-m', '--max-size', default=100, - help='Max size of the listed pipeline versions.') + help=parsing.get_param_descr(client.Client.list_pipeline_versions, + 'page_size')) @click.option( '--sort-by', default='created_at desc', - help="Can be '[field_name]', '[field_name] desc'. For example, 'name desc'." -) + help=parsing.get_param_descr(client.Client.list_pipeline_versions, + 'sort_by')) @click.option( '--filter', - help=( - 'filter: A url-encoded, JSON-serialized Filter protocol buffer ' - '(see [filter.proto](https://github.com/kubeflow/pipelines/blob/master/backend/api/filter.proto)).' - )) + help=parsing.get_param_descr(client.Client.list_pipeline_versions, + 'filter')) @click.pass_context def list_versions(ctx: click.Context, pipeline_id: str, page_token: str, max_size: int, sort_by: str, filter: str): - """List versions of an uploaded KFP pipeline.""" + """List versions of a pipeline.""" client = ctx.obj['client'] output_format = ctx.obj['output'] @@ -162,7 +191,7 @@ def list_versions(ctx: click.Context, pipeline_id: str, page_token: str, @click.argument('version-id') @click.pass_context def delete_version(ctx: click.Context, version_id: str): - """Delete pipeline version. + """Delete a version of a pipeline. Args: version_id: id of the pipeline version. @@ -187,7 +216,7 @@ def delete_version(ctx: click.Context, version_id: str): @click.argument('pipeline-id') @click.pass_context def get(ctx: click.Context, pipeline_id: str): - """Get detailed information about an uploaded KFP pipeline.""" + """Get information about a pipeline.""" client = ctx.obj['client'] output_format = ctx.obj['output'] @@ -199,7 +228,7 @@ def get(ctx: click.Context, pipeline_id: str): @click.argument('version-id') @click.pass_context def get_version(ctx: click.Context, version_id: str): - """Get detailed information about an uploaded KFP pipeline version.""" + """Get information about a version of a pipeline.""" client = ctx.obj['client'] output_format = ctx.obj['output'] @@ -211,7 +240,7 @@ def get_version(ctx: click.Context, version_id: str): @click.argument('pipeline-id') @click.pass_context def delete(ctx: click.Context, pipeline_id: str): - """Delete an uploaded KFP pipeline.""" + """Delete a pipeline.""" client = ctx.obj['client'] confirmation = f'Are you sure you want to delete pipeline {pipeline_id}?' diff --git a/sdk/python/kfp/cli/recurring_run.py b/sdk/python/kfp/cli/recurring_run.py index 78a1452f438..18f68e7d8ed 100644 --- a/sdk/python/kfp/cli/recurring_run.py +++ b/sdk/python/kfp/cli/recurring_run.py @@ -17,64 +17,86 @@ import click import kfp_server_api +from kfp import client from kfp.cli.output import OutputFormat from kfp.cli.output import print_output +from kfp.cli.utils import parsing @click.group() def recurring_run(): - """Manage recurring-run resources.""" + """Manage recurring run resources.""" pass +either_option_required = 'Either --experiment-id or --experiment-name is required.' + + @recurring_run.command() @click.option( '--catchup/--no-catchup', - help='Whether the recurring run should catch up if behind schedule.', - type=bool) + type=bool, + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'no_catchup'), +) @click.option( '--cron-expression', - help='A cron expression representing a set of times, using 6 space-separated fields, e.g. "0 0 9 ? * 2-6".' -) -@click.option('--description', help='The description of the recurring run.') + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'cron_expression')) +@click.option( + '--description', + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'description')) @click.option( '--enable-caching/--disable-caching', - help='Optional. Whether or not to enable caching for the run.', - type=bool) + type=bool, + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'enable_caching'), +) @click.option( '--enabled/--disabled', - help='A bool indicating whether the recurring run is enabled or disabled.', - type=bool) + type=bool, + help=parsing.get_param_descr(client.Client.create_recurring_run, 'enabled')) @click.option( '--end-time', - help='The RFC3339 time string of the time when to end the job.') + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'end_time')) @click.option( '--experiment-id', - help='The ID of the experiment to create the recurring run under, can only supply either an experiment ID or name.' + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'experiment_id') + ' ' + either_option_required ) @click.option( '--experiment-name', - help='The name of the experiment to create the recurring run under, can only supply either an experiment ID or name.' -) -@click.option('--job-name', help='The name of the recurring run.') + help='The name of the experiment to create the recurring run under.' + ' ' + + either_option_required) +@click.option( + '--job-name', + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'job_name')) @click.option( '--interval-second', - help='Integer indicating the seconds between two recurring runs in for a periodic schedule.' -) + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'interval_second')) @click.option( '--max-concurrency', - help='Integer indicating how many jobs can be run in parallel.', - type=int) + type=int, + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'max_concurrency'), +) @click.option( '--pipeline-id', - help='The ID of the pipeline to use to create the recurring run.') + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'pipeline_id'), +) @click.option( '--pipeline-package-path', - help='Local path of the pipeline package(the filename should end with one of the following .tar.gz, .tgz, .zip, .yaml, .yml).' -) + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'pipeline_package_path')) @click.option( '--start-time', - help='The RFC3339 time string of the time when to start the job.') + help=parsing.get_param_descr(client.Client.create_recurring_run, + 'start_time')) @click.option('--version-id', help='The id of a pipeline version.') @click.argument('args', nargs=-1) @click.pass_context @@ -100,8 +122,7 @@ def create(ctx: click.Context, output_format = ctx.obj['output'] if (experiment_id is None) == (experiment_name is None): - raise ValueError( - 'Either --experiment-id or --experiment-name is required') + raise ValueError(either_option_required) if not experiment_id: experiment = client.create_experiment(experiment_name) experiment_id = experiment.id @@ -133,25 +154,26 @@ def create(ctx: click.Context, @click.option( '-e', '--experiment-id', - help='Parent experiment ID of listed recurring runs.') + help=parsing.get_param_descr(client.Client.list_recurring_runs, + 'experiment_id')) @click.option( - '--page-token', default='', help='Token for starting of the page.') + '--page-token', + default='', + help=parsing.get_param_descr(client.Client.list_recurring_runs, + 'page_token')) @click.option( '-m', '--max-size', default=100, - help='Max size of the listed recurring runs.') + help=parsing.get_param_descr(client.Client.list_recurring_runs, + 'page_size')) @click.option( '--sort-by', default='created_at desc', - help="Can be '[field_name]', '[field_name] desc'. For example, 'name desc'." -) + help=parsing.get_param_descr(client.Client.list_recurring_runs, 'sort_by')) @click.option( '--filter', - help=( - 'filter: A url-encoded, JSON-serialized Filter protocol buffer ' - '(see [filter.proto](https://github.com/kubeflow/pipelines/blob/master/backend/api/filter.proto)).' - )) + help=parsing.get_param_descr(client.Client.list_recurring_runs, 'filter')) @click.pass_context def list(ctx: click.Context, experiment_id: str, page_token: str, max_size: int, sort_by: str, filter: str): @@ -179,7 +201,7 @@ def list(ctx: click.Context, experiment_id: str, page_token: str, max_size: int, @click.argument('job-id') @click.pass_context def get(ctx: click.Context, job_id: str): - """Get detailed information about an experiment.""" + """Get information about a recurring run.""" client = ctx.obj['client'] output_format = ctx.obj['output'] @@ -214,7 +236,7 @@ def enable(ctx: click.Context, job_id: str): @click.argument('job-id') @click.pass_context def disable(ctx: click.Context, job_id: str): - """Enable a recurring run.""" + """Disable a recurring run.""" client = ctx.obj['client'] client.disable_job(job_id=job_id) click.echo(f'Disabled job {job_id}.') diff --git a/sdk/python/kfp/cli/run.py b/sdk/python/kfp/cli/run.py index 57675418451..eed582861f5 100644 --- a/sdk/python/kfp/cli/run.py +++ b/sdk/python/kfp/cli/run.py @@ -24,36 +24,39 @@ from kfp import client from kfp.cli.output import OutputFormat from kfp.cli.output import print_output +from kfp.cli.utils import parsing @click.group() def run(): - """manage run resources.""" + """Manage run resources.""" pass @run.command() @click.option( - '-e', '--experiment-id', help='Parent experiment ID of listed runs.') + '-e', + '--experiment-id', + help=parsing.get_param_descr(client.Client.list_runs, 'experiment_id')) @click.option( - '--page-token', default='', help='Token for starting of the page.') + '--page-token', + default='', + help=parsing.get_param_descr(client.Client.list_runs, 'page_token')) @click.option( - '-m', '--max-size', default=100, help='Max size of the listed runs.') + '-m', + '--max-size', + default=100, + help=parsing.get_param_descr(client.Client.list_runs, 'page_size')) @click.option( '--sort-by', default='created_at desc', - help="Can be '[field_name]', '[field_name] desc'. For example, 'name desc'." -) + help=parsing.get_param_descr(client.Client.list_runs, 'sort_by')) @click.option( - '--filter', - help=( - 'filter: A url-encoded, JSON-serialized Filter protocol buffer ' - '(see [filter.proto](https://github.com/kubeflow/pipelines/blob/master/backend/api/filter.proto)).' - )) + '--filter', help=parsing.get_param_descr(client.Client.list_runs, 'filter')) @click.pass_context def list(ctx: click.Context, experiment_id: str, page_token: str, max_size: int, sort_by: str, filter: str): - """list recent KFP runs.""" + """List pipeline runs.""" client = ctx.obj['client'] output_format = ctx.obj['output'] response = client.list_runs( @@ -78,13 +81,20 @@ def list(ctx: click.Context, experiment_id: str, page_token: str, max_size: int, '--experiment-name', required=True, help='Experiment name of the run.') -@click.option('-r', '--run-name', help='Name of the run.') +@click.option( + '-r', + '--run-name', + help=parsing.get_param_descr(client.Client.run_pipeline, 'job_name')) @click.option( '-f', '--package-file', type=click.Path(exists=True, dir_okay=False), - help='Path of the pipeline package file.') -@click.option('-p', '--pipeline-id', help='ID of the pipeline template.') + help=parsing.get_param_descr(client.Client.run_pipeline, + 'pipeline_package_path')) +@click.option( + '-p', + '--pipeline-id', + help=parsing.get_param_descr(client.Client.run_pipeline, 'pipeline_id')) @click.option('-n', '--pipeline-name', help='Name of the pipeline template.') @click.option( '-w', @@ -92,7 +102,10 @@ def list(ctx: click.Context, experiment_id: str, page_token: str, max_size: int, is_flag=True, default=False, help='Watch the run status until it finishes.') -@click.option('-v', '--version', help='ID of the pipeline version.') +@click.option( + '-v', + '--version', + help=parsing.get_param_descr(client.Client.run_pipeline, 'version_id')) @click.option( '-t', '--timeout', @@ -104,7 +117,7 @@ def list(ctx: click.Context, experiment_id: str, page_token: str, max_size: int, def submit(ctx: click.Context, experiment_name: str, run_name: str, package_file: str, pipeline_id: str, pipeline_name: str, watch: bool, timeout: int, version: str, args: List[str]): - """submit a KFP run.""" + """Submit a pipeline run.""" client = ctx.obj['client'] namespace = ctx.obj['namespace'] output_format = ctx.obj['output'] @@ -124,11 +137,11 @@ def submit(ctx: click.Context, experiment_name: str, run_name: str, experiment = client.create_experiment(experiment_name) run = client.run_pipeline( - experiment.id, - run_name, - package_file, - arg_dict, - pipeline_id, + experiment_id=experiment.id, + job_name=run_name, + pipeline_package_path=package_file, + params=arg_dict, + pipeline_id=pipeline_id, version_id=version) if timeout > 0: _wait_for_run_completion(client, run.id, timeout, output_format) @@ -152,7 +165,7 @@ def submit(ctx: click.Context, experiment_name: str, run_name: str, @click.argument('run-id') @click.pass_context def get(ctx: click.Context, watch: bool, detail: bool, run_id: str): - """display the details of a KFP run.""" + """Get information about a pipeline run.""" client = ctx.obj['client'] namespace = ctx.obj['namespace'] output_format = ctx.obj['output'] @@ -164,7 +177,7 @@ def get(ctx: click.Context, watch: bool, detail: bool, run_id: str): @click.argument('run-id') @click.pass_context def archive(ctx: click.Context, run_id: str): - """Archive a run.""" + """Archive a pipeline run.""" client = ctx.obj['client'] if run_id is None: click.echo('You must provide a run-id.', err=True) @@ -183,7 +196,7 @@ def archive(ctx: click.Context, run_id: str): @click.argument('run-id') @click.pass_context def unarchive(ctx: click.Context, run_id: str): - """Unarchive a run.""" + """Unarchive a pipeline run.""" client = ctx.obj['client'] if run_id is None: click.echo('You must provide a run-id.', err=True) @@ -202,7 +215,7 @@ def unarchive(ctx: click.Context, run_id: str): @click.argument('run-id') @click.pass_context def delete(ctx: click.Context, run_id: str): - """Delete a run.""" + """Delete a pipeline run.""" confirmation = f'Are you sure you want to delete run {run_id}?' if not click.confirm(confirmation): diff --git a/sdk/python/kfp/cli/utils/parsing.py b/sdk/python/kfp/cli/utils/parsing.py new file mode 100644 index 00000000000..87be84b289a --- /dev/null +++ b/sdk/python/kfp/cli/utils/parsing.py @@ -0,0 +1,69 @@ +# 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 re +from typing import Callable + + +def get_param_descr(fn: Callable, param_name: str) -> str: + """Extracts the description of a parameter from the docstring of a function + or method. Docstring must conform to Google Style (https://sphinxcontrib- + napoleon.readthedocs.io/en/latest/example_google.html). + + Args: + fn (Callable): The function of method with a __doc__ docstring implemented. + param_name (str): The parameter for which to extract the description. + + Raises: + ValueError: If docstring is not found or parameter is not found in docstring. + + Returns: + str: The description of the parameter. + """ + docstring = fn.__doc__ + + if docstring is None: + raise ValueError( + f'Could not find parameter {param_name} in docstring of {fn}') + lines = docstring.splitlines() + + # collect all lines beginning after args, also get indentation space_chars + for i, line in enumerate(lines): + if line.lstrip().startswith('Args:'): + break + + lines = lines[i + 1:] + + first_already_found = False + return_lines = [] + + # allow but don't require type in docstring + first_line_args_regex = rf'^{param_name}( \(.*\))?: ' + for line in lines: + if not first_already_found and re.match(first_line_args_regex, + line.lstrip()): + new_line = re.sub(first_line_args_regex, '', line.strip()) + return_lines.append(new_line) + first_already_found = True + first_indentation_level = len(line) - len(line.lstrip()) + continue + + if first_already_found: + indentation_level = len(line) - len(line.lstrip()) + if indentation_level <= first_indentation_level: + return ' '.join(return_lines) + else: + return_lines.append(line.strip()) + raise ValueError( + f'Could not find parameter {param_name} in docstring of {fn}') diff --git a/sdk/python/kfp/cli/utils/parsing_test.py b/sdk/python/kfp/cli/utils/parsing_test.py new file mode 100644 index 00000000000..17a7c3aafdd --- /dev/null +++ b/sdk/python/kfp/cli/utils/parsing_test.py @@ -0,0 +1,136 @@ +# 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 + +from kfp import client +from kfp.cli.utils import parsing + + +class MyClass: + """Class init. + + Args: + a (str): String a. + b (str): String b. + """ + + def __init__(self, a: str, b: str): + """Class init. + + Args: + a (str): String a. + b (str): String b. + """ + self.a = a + self.b = b + + def method(self, a: str, b: str): + """My method. + + Args: + a (str): String a. + b (str): String b. + + Returns: + str: Concat str. + """ + return a + b + + +class TestGetParamDescr(unittest.TestCase): + + def test_function(self): + + def fn(a: str, b: str): + """My fn. + + Args: + a (str): String a. + b (str): String b. + + Returns: + str: Concat str. + """ + return a + b + + a_descr = parsing.get_param_descr(fn, 'a') + self.assertEqual(a_descr, 'String a.') + b_descr = parsing.get_param_descr(fn, 'b') + self.assertEqual(b_descr, 'String b.') + + def test_function_without_type_info(self): + + def fn(a: str, b: str): + """My fn. + + Args: + a: String a. + b: String b. + + Returns: + str: Concat str. + """ + return a + b + + a_descr = parsing.get_param_descr(fn, 'a') + self.assertEqual(a_descr, 'String a.') + b_descr = parsing.get_param_descr(fn, 'b') + self.assertEqual(b_descr, 'String b.') + + def test_class(self): + + class_doc_a = parsing.get_param_descr(MyClass, 'a') + self.assertEqual(class_doc_a, 'String a.') + class_doc_b = parsing.get_param_descr(MyClass, 'b') + self.assertEqual(class_doc_b, 'String b.') + + init_a = parsing.get_param_descr(MyClass.__init__, 'a') + self.assertEqual(init_a, 'String a.') + init_b = parsing.get_param_descr(MyClass.__init__, 'b') + self.assertEqual(init_b, 'String b.') + + method_a = parsing.get_param_descr(MyClass.method, 'a') + self.assertEqual(method_a, 'String a.') + method_b = parsing.get_param_descr(MyClass.method, 'b') + self.assertEqual(method_b, 'String b.') + + def test_instance(self): + instance = MyClass('a', 'b') + + inst_doc_a = parsing.get_param_descr(instance, 'a') + self.assertEqual(inst_doc_a, 'String a.') + inst_doc_b = parsing.get_param_descr(instance, 'b') + self.assertEqual(inst_doc_b, 'String b.') + + init_a = parsing.get_param_descr(instance.__init__, 'a') + self.assertEqual(init_a, 'String a.') + init_b = parsing.get_param_descr(instance.__init__, 'b') + self.assertEqual(init_b, 'String b.') + + method_a = parsing.get_param_descr(instance.method, 'a') + self.assertEqual(method_a, 'String a.') + method_b = parsing.get_param_descr(instance.method, 'b') + self.assertEqual(method_b, 'String b.') + + def test_multiline(self): + host_descr = parsing.get_param_descr(client.Client, 'host') + + self.assertEqual( + host_descr, + "The host name to use to talk to Kubeflow Pipelines. If not set, the in-cluster service DNS name will be used, which only works if the current environment is a pod in the same cluster (such as a Jupyter instance spawned by Kubeflow's JupyterHub). Set the host based on https://www.kubeflow.org/docs/components/pipelines/sdk/connect-api/" + ) + + +if __name__ == '__main__': + unittest.main() diff --git a/sdk/python/kfp/client/client.py b/sdk/python/kfp/client/client.py index 4ca3a6f7e97..6a5eeb263fb 100644 --- a/sdk/python/kfp/client/client.py +++ b/sdk/python/kfp/client/client.py @@ -53,7 +53,7 @@ class Client: - """The API Client for KubeFlow Pipeline. + """The API Client for KubeFlow Pipelines. Args: host: The host name to use to talk to Kubeflow Pipelines. If not set, @@ -544,8 +544,8 @@ def get_experiment(self, Either experiment_id or experiment_name is required. Args: - experiment_id: Id of the experiment. (Optional) - experiment_name: Name of the experiment. (Optional) + experiment_id (Optional): Id of the experiment. + experiment_name (Optional): Name of the experiment. namespace: Kubernetes namespace where the experiment was created. For single user deployment, leave it as None; For multi user, input the namespace where the user is authorized. @@ -734,13 +734,13 @@ def run_pipeline( If only pipeline_id is specified, the default version of this pipeline is used to create the run. pipeline_root: The root path of the pipeline outputs. - enable_caching: Optional. Whether or not to enable caching for the + enable_caching (Optional): . Whether or not to enable caching for the run. If not set, defaults to the compile time settings, which are True for all tasks by default, while users may specify different caching options for individual tasks. If set, the setting applies to all tasks in the pipeline (overrides the compile time settings). - service_account: Optional. Specifies which Kubernetes service + service_account (Optional): Specifies which Kubernetes service account this run uses. Returns: @@ -832,7 +832,7 @@ def create_recurring_run( Args: experiment_id: The string id of an experiment. job_name: Name of the job. - description: An optional job description. + description (Optional): Job description. start_time: The RFC3339 time string of the time when to start the job. end_time: The RFC3339 time string of the time when to end the job. @@ -864,13 +864,13 @@ def create_recurring_run( enabled: A bool indicating whether the recurring run is enabled or disabled. pipeline_root: The root path of the pipeline outputs. - enable_caching: Optional. Whether or not to enable caching for the + enable_caching (Optional): Whether or not to enable caching for the run. If not set, defaults to the compile time settings, which are True for all tasks by default, while users may specify different caching options for individual tasks. If set, the setting applies to all tasks in the pipeline (overrides the compile time settings). - service_account: Optional. Specifies which Kubernetes service + service_account (Optional): Specifies which Kubernetes service account this recurring run uses. Returns: A Job object. Most important field is id. @@ -941,7 +941,7 @@ def _create_job_config( If both pipeline_id and version_id are specified, version_id will take precedence. If only pipeline_id is specified, the default version of this pipeline is used to create the run. - enable_caching: Optional. Whether or not to enable caching for the + enable_caching (Optional): Whether or not to enable caching for the run. If not set, defaults to the compile time settings, which are True for all tasks by default, while users may specify different caching options for individual tasks. If set, the @@ -1019,19 +1019,19 @@ def create_run_from_pipeline_func( pipeline_func: A function that describes a pipeline by calling components and composing them into execution graph. arguments: Arguments to the pipeline function provided as a dict. - run_name: Optional. Name of the run to be shown in the UI. - experiment_name: Optional. Name of the experiment to add the run to. + run_name (Optional): Name of the run to be shown in the UI. + experiment_name (Optional): Name of the experiment to add the run to. namespace: Kubernetes namespace where the pipeline runs are created. For single user deployment, leave it as None; For multi user, input a namespace where the user is authorized pipeline_root: The root path of the pipeline outputs. - enable_caching: Optional. Whether or not to enable caching for the + enable_caching (Optional): Whether or not to enable caching for the run. If not set, defaults to the compile time settings, which are True for all tasks by default, while users may specify different caching options for individual tasks. If set, the setting applies to all tasks in the pipeline (overrides the compile time settings). - service_account: Optional. Specifies which Kubernetes service + service_account (Optional): Specifies which Kubernetes service account this run uses. """ #TODO: Check arguments against the pipeline function @@ -1076,19 +1076,19 @@ def create_run_from_pipeline_package( Args: pipeline_file: A compiled pipeline package file. arguments: Arguments to the pipeline function provided as a dict. - run_name: Optional. Name of the run to be shown in the UI. - experiment_name: Optional. Name of the experiment to add the run to. - namespace: Kubernetes namespace where the pipeline runs are created. + run_name (Optional): Name of the run to be shown in the UI. + experiment_name (Optional): Name of the experiment to add the run to. + namespace (Optional): Kubernetes namespace where the pipeline runs are created. For single user deployment, leave it as None; For multi user, input a namespace where the user is authorized - pipeline_root: The root path of the pipeline outputs. - enable_caching: Optional. Whether or not to enable caching for the + pipeline_root (Optional): The root path of the pipeline outputs. + enable_caching (Optional): Whether or not to enable caching for the run. If not set, defaults to the compile time settings, which are True for all tasks by default, while users may specify different caching options for individual tasks. If set, the setting applies to all tasks in the pipeline (overrides the compile time settings). - service_account: Optional. Specifies which Kubernetes service + service_account (Optional): Specifies which Kubernetes service account this run uses. """ @@ -1385,8 +1385,8 @@ def upload_pipeline( Args: pipeline_package_path: Local path to the pipeline package. - pipeline_name: Optional. Name of the pipeline to be shown in the UI. - description: Optional. Description of the pipeline to be shown in + pipeline_name (Optional): Name of the pipeline to be shown in the UI. + description (Optional): Description of the pipeline to be shown in the UI. Returns: @@ -1416,9 +1416,9 @@ def upload_pipeline_version( pipeline_package_path: Local path to the pipeline package. pipeline_version_name: Name of the pipeline version to be shown in the UI. - pipeline_id: Optional. Id of the pipeline. - pipeline_name: Optional. Name of the pipeline. - description: Optional. Description of the pipeline version to be + pipeline_id (Optional): Id of the pipeline. + pipeline_name (Optional): Name of the pipeline. + description (Optional): Description of the pipeline version to be shown in the UI. Returns: