From 0c3efc3bf81832de59c789e031f4f5f1f2a901ef Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Wed, 18 Mar 2026 16:05:29 +0100 Subject: [PATCH] WIP fixes #1315 --- CHANGES/1315.bugfix | 1 + src/pulp_cli/generic.py | 13 ++++++++ src/pulpcore/cli/ansible/remote.py | 7 ++-- src/pulpcore/cli/container/content.py | 45 +++++++++----------------- src/pulpcore/cli/core/content_guard.py | 20 ++++++------ src/pulpcore/cli/file/repository.py | 44 ++++++++++++++----------- src/pulpcore/cli/rpm/content.py | 14 +++----- tests/test_help_pages.py | 1 + 8 files changed, 73 insertions(+), 72 deletions(-) create mode 100644 CHANGES/1315.bugfix 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..024f3c89e 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)) @@ -483,6 +486,16 @@ def pulp_command( return click.command(name=name, cls=PulpCommand, **kwargs) +def option_processor( + *, callback: t.Callable[[click.Context], None] +) -> t.Callable[[PulpCommand], PulpCommand]: + def _inner(cmd: PulpCommand) -> PulpCommand: + cmd.option_processors.append(callback) + return cmd + + return _inner + + def pulp_group(name: str | None = None, **kwargs: t.Any) -> t.Callable[[_AnyCallable], PulpGroup]: """ Pulp command group factory. 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/container/content.py b/src/pulpcore/cli/container/content.py index 7f9ba4b50..a3b9e03cb 100644 --- a/src/pulpcore/cli/container/content.py +++ b/src/pulpcore/cli/container/content.py @@ -1,9 +1,8 @@ import gettext -import typing as t 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 +10,12 @@ ) from pulp_cli.generic import ( - GroupOption, PulpCLIContext, content_filter_options, href_option, label_command, list_command, + option_processor, pass_pulp_context, pulp_group, pulp_option, @@ -26,14 +25,18 @@ _ = 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) -> None: + lookup = { + key: value + for key, value in ((key, ctx.params.pop(key, None)) for key in ["name", "digest"]) + if value is not None + } + if lookup: + 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(lookup) != 2: + raise click.UsageError(_("Both 'name' and 'digest' are needed to describe a tag.")) + entity_ctx.entity = lookup @pulp_group() @@ -98,33 +101,15 @@ 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_processor(callback=_content_callback), ] 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..c7f41ecc3 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_processor, pass_pulp_context, pass_repository_context, pulp_group, @@ -60,12 +60,20 @@ ) -def _content_callback(ctx: click.Context, param: click.Parameter, value: 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 +def _content_callback(ctx: click.Context) -> t.Any: + lookup = { + key: value + for key, value in ((key, ctx.params.pop(key, None)) for key in ["sha256", "relative_path"]) + if value is not None + } + if lookup: + if len(lookup) != 2: + raise click.UsageError( + _("Both 'sha256' and 'relative-path' must be provided to specify a file content.") + ) + entity_ctx = ctx.find_object(PulpFileContentContext) + assert entity_ctx is not None + entity_ctx.entity = lookup CONTENT_LIST_SCHEMA = s.Schema([{"sha256": str, "relative_path": s.And(str, len)}]) @@ -119,15 +127,10 @@ 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"], - callback=_content_callback, - ), +file_content_options = [ + click.option("--sha256"), + click.option("--relative-path"), + option_processor(callback=_content_callback), ] content_json_callback = create_content_json_callback( PulpFileContentContext, schema=CONTENT_LIST_SCHEMA @@ -155,7 +158,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 +176,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],