diff --git a/CHANGES/1315.bugfix b/CHANGES/1315.bugfix new file mode 100644 index 000000000..c65e8e7cb --- /dev/null +++ b/CHANGES/1315.bugfix @@ -0,0 +1 @@ +Fixed warnings about parameters being used multiple times. diff --git a/src/pulp_cli/generic.py b/src/pulp_cli/generic.py index 55730ba1d..03d95fbd1 100644 --- a/src/pulp_cli/generic.py +++ b/src/pulp_cli/generic.py @@ -399,6 +399,7 @@ def __init__( ): self.allowed_with_contexts = allowed_with_contexts self.needs_plugins = needs_plugins + self.option_processors: list[t.Callable[[click.Context], None]] = [] super().__init__(*args, **kwargs) def invoke(self, ctx: click.Context) -> t.Any: @@ -408,6 +409,8 @@ def invoke(self, ctx: click.Context) -> t.Any: assert pulp_ctx is not None for plugin_requirement in self.needs_plugins: pulp_ctx.needs_plugin(plugin_requirement) + for processor in self.option_processors: + processor(ctx) return super().invoke(ctx) except PulpException as e: raise click.ClickException(str(e)) @@ -471,7 +474,9 @@ def list_commands(self, ctx: click.Context) -> list[str]: def pulp_command( - name: str | None = None, **kwargs: t.Any + name: str | None = None, + cls: t.Type[PulpCommand] = PulpCommand, + **kwargs: t.Any, ) -> t.Callable[[_AnyCallable], PulpCommand]: """ Pulp command factory. @@ -480,17 +485,21 @@ def pulp_command( `allowed_with_contexts`. It allows rendering the docstring with the values of `ENTITY` and `ENTITIES` from the closest entity context. """ - return click.command(name=name, cls=PulpCommand, **kwargs) + return click.command(name=name, cls=cls, **kwargs) -def pulp_group(name: str | None = None, **kwargs: t.Any) -> t.Callable[[_AnyCallable], PulpGroup]: +def pulp_group( + name: str | None = None, + cls: t.Type[PulpGroup] = PulpGroup, + **kwargs: t.Any, +) -> t.Callable[[_AnyCallable], PulpGroup]: """ Pulp command group factory. Creates a click compatible group command that selects subcommands based on `allowed_with_contexts` and creates `PulpCommand` subcommands by default. """ - return click.group(name=name, cls=PulpGroup, **kwargs) + return click.group(name=name, cls=cls, **kwargs) class PulpOption(click.Option): @@ -631,7 +640,9 @@ def _version_callback(ctx: click.Context, param: click.Parameter, value: int | N return value -def load_file_wrapper(handler: t.Callable[[click.Context, click.Parameter, str], t.Any]) -> t.Any: +def load_file_wrapper( + handler: t.Callable[[click.Context, click.Parameter, str], t.Any], +) -> t.Any: """ A wrapper that is used for chaining or decorating callbacks that manipulate input data. @@ -799,7 +810,11 @@ def remote_header_callback( # Decorator common options -def pulp_option(*args: t.Any, **kwargs: t.Any) -> t.Callable[[FC], FC]: +def pulp_option( + *args: t.Any, + cls: t.Type[PulpOption] = PulpOption, + **kwargs: t.Any, +) -> t.Callable[[FC], FC]: """ Factory of [`PulpOption`][pulp_cli.generic.PulpOption] objects. @@ -820,8 +835,72 @@ def pulp_option(*args: t.Any, **kwargs: t.Any) -> t.Callable[[FC], FC]: ) ``` """ - kwargs["cls"] = PulpOption - return click.option(*args, **kwargs) + return click.option(*args, cls=cls, **kwargs) + + +def option_processor( + *, callback: t.Callable[[click.Context], None] +) -> t.Callable[[PulpCommand], PulpCommand]: + """ + Add a callback i.e. for option processing at a rather generic place before calling the command. + """ + + def _inner(cmd: PulpCommand) -> PulpCommand: + cmd.option_processors.append(callback) + return cmd + + return _inner + + +def option_group( + name: str, + options: t.List[str], + /, + callback: t.Callable[[click.Context, t.Dict[str, t.Any]], t.Any] | None = None, + require_all: bool = True, + expose_value: bool = True, +) -> t.Callable[[PulpCommand], PulpCommand]: + def _group_callback(ctx: click.Context) -> None: + """ + Group a list of options into a group represented as a dictionary. + This allows to add a `callback` function for further processing. + `expose_value` allows to hide the value from the command callback. + """ + value = {k: v for k, v in ((k, ctx.params.pop(k, None)) for k in options) if v is not None} + if value: + if require_all and (missing_options := set(options) - set(value.keys())): + raise click.UsageError( + _( + "Illegal usage, please specify all options in the group: {option_list}\n" + "missing: {missing_options}" + ).format( + option_list=", ".join(options), + missing_options=", ".join(missing_options), + ) + ) + + if callback is not None: + value = callback(ctx, value) + if expose_value: + ctx.params[name] = value + + def _inner(cmd: PulpCommand) -> PulpCommand: + for param in cmd.params: + if param.name in options: + other_options = [o for o in options if o != param.name] + if other_options: + help_text = ( + (getattr(param, "help") or "") + + "\n" + + _("Option is grouped with {options}.").format( + options=", ".join(other_options) + ) + ).strip() + setattr(param, "help", help_text) + cmd.option_processors.append(_group_callback) + return cmd + + return _inner def resource_lookup_option(*args: t.Any, **kwargs: t.Any) -> t.Callable[[FC], FC]: @@ -1013,7 +1092,11 @@ def _option_callback( _( "The type '{plugin}:{resource_type}' " "does not support the '{capability}' capability." - ).format(plugin=plugin, resource_type=resource_type, capability=capability) + ).format( + plugin=plugin, + resource_type=resource_type, + capability=capability, + ) ) if parent_resource_lookup: @@ -1044,7 +1127,7 @@ def _multi_option_callback( "{plugin_default}{type_default}{multiple_note}" ).format( plugin_form=_("[:]") if default_plugin else _(":"), - type_form=_("[:]") if default_type else _(":"), + type_form=(_("[:]") if default_type else _(":")), lookup_key=lookup_key, plugin_default=( _("'' defaults to {plugin}. ").format(plugin=default_plugin) @@ -1376,7 +1459,9 @@ def _type_callback(ctx: click.Context, param: click.Parameter, value: str | None help=_("total number of simultaneous connections"), ), click.option( - "--rate-limit", type=int_or_empty, help=_("limit download rate in requests per second") + "--rate-limit", + type=int_or_empty, + help=_("limit download rate in requests per second"), ), click.option("--sock-connect-timeout", type=float_or_empty), click.option("--sock-read-timeout", type=float_or_empty), diff --git a/src/pulpcore/cli/ansible/content.py b/src/pulpcore/cli/ansible/content.py index cd475a2d2..f93b02d30 100644 --- a/src/pulpcore/cli/ansible/content.py +++ b/src/pulpcore/cli/ansible/content.py @@ -8,18 +8,22 @@ PulpAnsibleRepositoryContext, PulpAnsibleRoleContext, ) -from pulp_glue.common.context import PluginRequirement, PulpContentContext, PulpRepositoryContext +from pulp_glue.common.context import ( + PluginRequirement, + PulpContentContext, + PulpRepositoryContext, +) from pulp_glue.common.i18n import get_translation from pulp_glue.core.context import PulpArtifactContext from pulp_cli.generic import ( - GroupOption, PulpCLIContext, chunk_size_callback, content_filter_options, href_option, label_command, list_command, + option_group, pass_content_context, pass_pulp_context, pulp_group, @@ -38,7 +42,7 @@ signature_context = (PulpAnsibleCollectionVersionSignatureContext,) -def _content_callback(ctx: click.Context, param: click.Parameter, value: t.Any) -> t.Any: +def _content_callback(ctx: click.Context, value: t.Any) -> t.Any: if value: entity_ctx = ctx.find_object(PulpContentContext) assert entity_ctx is not None @@ -85,9 +89,15 @@ def content(ctx: click.Context, pulp_ctx: PulpCLIContext, /, content_type: str) list_options = [ pulp_option("--name", help=_("Name of {entity}"), allowed_with_contexts=content_context), pulp_option( - "--namespace", help=_("Namespace of {entity}"), allowed_with_contexts=content_context + "--namespace", + help=_("Namespace of {entity}"), + allowed_with_contexts=content_context, + ), + pulp_option( + "--version", + help=_("Version of {entity}"), + allowed_with_contexts=content_context, ), - pulp_option("--version", help=_("Version of {entity}"), allowed_with_contexts=content_context), pulp_option( "--latest", "is_highest", @@ -128,48 +138,52 @@ def content(ctx: click.Context, pulp_ctx: PulpCLIContext, /, content_type: str) ] lookup_options = [ - click.option( + pulp_option( "--name", help=_("Name of {entity}"), - group=["namespace", "version"], - expose_value=False, - allowed_with_contexts=(PulpAnsibleRoleContext, PulpAnsibleCollectionVersionContext), - cls=GroupOption, - callback=_content_callback, + allowed_with_contexts=( + PulpAnsibleRoleContext, + PulpAnsibleCollectionVersionContext, + ), ), - click.option( + pulp_option( "--namespace", help=_("Namespace of {entity}"), - group=["name", "version"], - expose_value=False, - allowed_with_contexts=(PulpAnsibleRoleContext, PulpAnsibleCollectionVersionContext), - cls=GroupOption, + allowed_with_contexts=( + PulpAnsibleRoleContext, + PulpAnsibleCollectionVersionContext, + ), ), - click.option( + pulp_option( "--version", help=_("Version of {entity}"), - group=["namespace", "name"], + allowed_with_contexts=( + PulpAnsibleRoleContext, + PulpAnsibleCollectionVersionContext, + ), + ), + option_group( + "content", + ["namespace", "name", "version"], expose_value=False, - allowed_with_contexts=(PulpAnsibleRoleContext, PulpAnsibleCollectionVersionContext), - cls=GroupOption, + callback=_content_callback, ), - click.option( + pulp_option( "--pubkey-fingerprint", help=_("Public key fingerprint of the {entity}"), - group=["collection"], - expose_value=False, allowed_with_contexts=signature_context, - callback=_content_callback, - cls=GroupOption, ), - click.option( + pulp_option( "--collection", "signed_collection", help=_("Collection of {entity}"), - group=["pubkey_fingerprint"], - expose_value=False, allowed_with_contexts=signature_context, - cls=GroupOption, + ), + option_group( + "signature_content", + ["signed_collection", "pubkey_fingerprint"], + expose_value=False, + callback=_content_callback, ), href_option, ] @@ -197,7 +211,10 @@ def content(ctx: click.Context, pulp_ctx: PulpCLIContext, /, content_type: str) allowed_with_contexts=content_context, ) @pulp_option( - "--name", help=_("Name of {entity}"), allowed_with_contexts=role_context, required=True + "--name", + help=_("Name of {entity}"), + allowed_with_contexts=role_context, + required=True, ) @pulp_option( "--namespace", diff --git a/src/pulpcore/cli/ansible/remote.py b/src/pulpcore/cli/ansible/remote.py index fbd0b0732..1adff58df 100644 --- a/src/pulpcore/cli/ansible/remote.py +++ b/src/pulpcore/cli/ansible/remote.py @@ -67,8 +67,6 @@ def remote(ctx: click.Context, pulp_ctx: PulpCLIContext, /, remote_type: str) -> nested_lookup_options = [remote_lookup_option] remote_options = [ click.option("--policy", help=_("policy to use when downloading")), -] -collection_options = [ pulp_option( "--requirements-file", callback=yaml_callback, @@ -96,9 +94,8 @@ def remote(ctx: click.Context, pulp_ctx: PulpCLIContext, /, remote_type: str) -> allowed_with_contexts=collection_context, ), ] -ansible_remote_options = remote_options + collection_options -create_options = common_remote_create_options + remote_options + ansible_remote_options -update_options = common_remote_update_options + remote_options + ansible_remote_options +create_options = common_remote_create_options + remote_options +update_options = common_remote_update_options + remote_options remote.add_command(list_command(decorators=remote_filter_options)) remote.add_command(show_command(decorators=lookup_options)) diff --git a/src/pulpcore/cli/ansible/repository.py b/src/pulpcore/cli/ansible/repository.py index c7046baae..80b5d3496 100644 --- a/src/pulpcore/cli/ansible/repository.py +++ b/src/pulpcore/cli/ansible/repository.py @@ -20,7 +20,6 @@ from pulp_glue.core.context import PulpSigningServiceContext from pulp_cli.generic import ( - GroupOption, PulpCLIContext, create_command, create_content_json_callback, @@ -32,6 +31,7 @@ load_json_callback, load_string_callback, name_option, + option_group, pass_pulp_context, pass_repository_context, pulp_group, @@ -72,7 +72,7 @@ ) -def _content_callback(ctx: click.Context, param: click.Parameter, value: t.Any) -> t.Any: +def _content_callback(ctx: click.Context, value: t.Any) -> t.Any: if value: ctx.obj.entity = value # The context is set by the type parameter on the content commands return value @@ -122,27 +122,23 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, /, repo_type: str) ] create_options = update_options + [click.option("--name", required=True)] content_options = [ - click.option( + pulp_option( "--name", help=_("Name of {entity}"), - group=["namespace", "version"], - expose_value=False, - cls=GroupOption, - callback=_content_callback, ), - click.option( + pulp_option( "--namespace", help=_("Namespace of {entity}"), - group=["name", "version"], - expose_value=False, - cls=GroupOption, ), - click.option( + pulp_option( "--version", help=_("Version of {entity}"), - group=["namespace", "name"], + ), + option_group( + "content", + ["namespace", "name", "version"], expose_value=False, - cls=GroupOption, + callback=_content_callback, ), href_option, ] diff --git a/src/pulpcore/cli/container/content.py b/src/pulpcore/cli/container/content.py index 7f9ba4b50..b03da1cd2 100644 --- a/src/pulpcore/cli/container/content.py +++ b/src/pulpcore/cli/container/content.py @@ -3,7 +3,7 @@ import click -from pulp_glue.common.context import PluginRequirement, PulpEntityContext +from pulp_glue.common.context import PluginRequirement, PulpContentContext from pulp_glue.container.context import ( PulpContainerBlobContext, PulpContainerManifestContext, @@ -11,12 +11,12 @@ ) from pulp_cli.generic import ( - GroupOption, PulpCLIContext, content_filter_options, href_option, label_command, list_command, + option_group, pass_pulp_context, pulp_group, pulp_option, @@ -26,14 +26,13 @@ _ = gettext.gettext -def _content_callback(ctx: click.Context, param: click.Parameter, value: t.Any) -> None: - if value is not None: - entity_ctx = ctx.find_object(PulpEntityContext) +def _content_callback(ctx: click.Context, value: t.Dict[str, t.Any]) -> None: + if value: + entity_ctx = ctx.find_object(PulpContentContext) assert entity_ctx is not None - if isinstance(entity_ctx, PulpContainerTagContext): - entity_ctx.entity = value - else: - entity_ctx.entity = {"digest": value} + if isinstance(entity_ctx, PulpContainerTagContext) and len(value) != 2: + raise click.UsageError(_("Both 'name' and 'digest' are needed to describe a tag.")) + entity_ctx.entity = value @pulp_group() @@ -64,7 +63,10 @@ def content(ctx: click.Context, pulp_ctx: PulpCLIContext, /, content_type: str) ), pulp_option("--name", "name", allowed_with_contexts=(PulpContainerTagContext,)), pulp_option( - "--name-in", "name__in", multiple=True, allowed_with_contexts=(PulpContainerTagContext,) + "--name-in", + "name__in", + multiple=True, + allowed_with_contexts=(PulpContainerTagContext,), ), pulp_option( "--digest", @@ -98,33 +100,21 @@ def content(ctx: click.Context, pulp_ctx: PulpCLIContext, /, content_type: str) lookup_options = [ pulp_option( "--digest", - expose_value=False, help=_("Digest associated with {entity}"), - callback=_content_callback, - allowed_with_contexts=(PulpContainerBlobContext, PulpContainerManifestContext), ), - click.option( - "--digest", - expose_value=False, - help=_("Digest associated with {entity}"), - callback=_content_callback, - allowed_with_contexts=(PulpContainerTagContext,), - group=[ - "name", - ], - cls=GroupOption, - ), - click.option( + pulp_option( "--name", - expose_value=False, help=_("Name of {entity}"), allowed_with_contexts=(PulpContainerTagContext,), - group=[ - "digest", - ], - cls=GroupOption, ), href_option, + option_group( + "content", + ["name", "digest"], + callback=_content_callback, + require_all=False, + expose_value=False, + ), ] content.add_command(list_command(decorators=list_options)) diff --git a/src/pulpcore/cli/core/content_guard.py b/src/pulpcore/cli/core/content_guard.py index b8da6fcc3..0b53833cc 100644 --- a/src/pulpcore/cli/core/content_guard.py +++ b/src/pulpcore/cli/core/content_guard.py @@ -203,7 +203,6 @@ def x509(ctx: click.Context, pulp_ctx: PulpCLIContext, /) -> None: certguard_create_options = common_create_options + [ - click.option("--description"), click.option( "--ca-certificate", help=_("a PEM encoded CA certificate or @file containing same"), @@ -212,14 +211,17 @@ def x509(ctx: click.Context, pulp_ctx: PulpCLIContext, /) -> None: ), ] -certguard_update_options = lookup_options + [ - click.option("--description"), - click.option( - "--ca-certificate", - help=_("a PEM encoded CA certificate or @file containing same"), - callback=load_string_callback, - ), -] +certguard_update_options = ( + lookup_options + + common_update_options + + [ + click.option( + "--ca-certificate", + help=_("a PEM encoded CA certificate or @file containing same"), + callback=load_string_callback, + ), + ] +) x509.add_command(list_command(decorators=filter_options)) diff --git a/src/pulpcore/cli/file/repository.py b/src/pulpcore/cli/file/repository.py index 4d412d9e7..2a788bf0f 100644 --- a/src/pulpcore/cli/file/repository.py +++ b/src/pulpcore/cli/file/repository.py @@ -17,7 +17,6 @@ ) from pulp_cli.generic import ( - GroupOption, PulpCLIContext, create_command, create_content_json_callback, @@ -29,6 +28,7 @@ list_command, load_file_wrapper, name_option, + option_group, pass_pulp_context, pass_repository_context, pulp_group, @@ -60,12 +60,11 @@ ) -def _content_callback(ctx: click.Context, param: click.Parameter, value: t.Any) -> t.Any: +def _content_callback(ctx: click.Context, value: t.Dict[str, t.Any]) -> t.Any: if value: - pulp_ctx = ctx.find_object(PulpCLIContext) - assert pulp_ctx is not None - ctx.obj = PulpFileContentContext(pulp_ctx, entity=value) - return value + entity_ctx = ctx.find_object(PulpFileContentContext) + assert entity_ctx is not None + entity_ctx.entity = value CONTENT_LIST_SCHEMA = s.Schema([{"sha256": str, "relative_path": s.And(str, len)}]) @@ -119,14 +118,14 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, /, repo_type: str) pulp_labels_option, ] create_options = update_options + [click.option("--name", required=True)] -file_options = [ - click.option("--sha256", cls=GroupOption, expose_value=False, group=["relative_path"]), - click.option( - "--relative-path", - cls=GroupOption, - expose_value=False, - group=["sha256"], +file_content_options = [ + click.option("--sha256"), + click.option("--relative-path"), + option_group( + "content", + ["sha256", "relative_path"], callback=_content_callback, + expose_value=False, ), ] content_json_callback = create_content_json_callback( @@ -155,7 +154,10 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, /, repo_type: str) repository.add_command( list_command( - decorators=[label_select_option, click.option("--name-startswith", "name__startswith")] + decorators=[ + label_select_option, + click.option("--name-startswith", "name__startswith"), + ] ) ) repository.add_command(show_command(decorators=lookup_options)) @@ -170,8 +172,8 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, /, repo_type: str) contexts={"file": PulpFileContentContext}, base_default_plugin="file", base_default_type="file", - add_decorators=file_options, - remove_decorators=file_options, + add_decorators=file_content_options, + remove_decorators=file_content_options, modify_decorators=modify_options, ) ) diff --git a/src/pulpcore/cli/rpm/content.py b/src/pulpcore/cli/rpm/content.py index fc9cc1c03..cf14c8444 100644 --- a/src/pulpcore/cli/rpm/content.py +++ b/src/pulpcore/cli/rpm/content.py @@ -354,15 +354,11 @@ def content() -> None: @pulp_option( "--file", type=click.File("rb"), - required=True, - help=_("An advisory JSON file."), - allowed_with_contexts=(PulpRpmAdvisoryContext,), -) -@pulp_option( - "--file", - type=click.File("rb"), - help=_("An RPM binary. One of --file or --directory is required."), - allowed_with_contexts=(PulpRpmPackageContext,), + help=_("A file to upload. One of --file or --directory is required."), + allowed_with_contexts=( + PulpRpmAdvisoryContext, + PulpRpmPackageContext, + ), required=False, ) @pulp_option( diff --git a/tests/test_help_pages.py b/tests/test_help_pages.py index 9ddc10c7a..d204ba371 100644 --- a/tests/test_help_pages.py +++ b/tests/test_help_pages.py @@ -51,6 +51,7 @@ def getter(self: t.Any) -> None: @pytest.mark.help_page(base_cmd=[]) +@pytest.mark.filterwarnings("error::UserWarning:click") def test_accessing_the_help_page_does_not_invoke_api( no_api: None, args: list[str],