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

It looks like option order can matter in unintuitive ways. #650

Closed
quba42 opened this issue Mar 7, 2023 · 3 comments · Fixed by #847
Closed

It looks like option order can matter in unintuitive ways. #650

quba42 opened this issue Mar 7, 2023 · 3 comments · Fixed by #847
Assignees
Labels
bug Something isn't working (template-set)

Comments

@quba42
Copy link
Contributor

quba42 commented Mar 7, 2023

Summary

While using Pulp CLI, I encountered the following:

$ oci-env pulp deb repository version destroy --version 1 --repository=${ENTITIES_NAME}
Error: A apt repository must be specified for this command.
$ oci-env pulp deb repository version destroy --repository=${ENTITIES_NAME} --version 1
Started background task /pulp/api/v3/tasks/51676f37-ea1f-4c97-b0e8-031f93577892/
.Done.

Steps to reproduce

  1. Create some repository, with some content in it.
  2. Delete a repository version using the --version and --repository flags in that order
  3. You get e.g.: Error: A apt repository must be specified for this command.

Expected behavior

The order the --version and --repository flags are given to a CLI command in should not matter.

Pulp and pulp-cli version info

# pulp --version
pulp3 command line interface, version 0.16.0
  "versions": [
    {
      "component": "core",
      "version": "3.22.0.dev",
      "package": "pulpcore"
    },
    {
      "component": "deb",
      "version": "2.21.0.dev",
      "package": "pulp_deb"
    },
    {
      "component": "file",
      "version": "1.12.0.dev",
      "package": "pulp-file"
    }
  ],

Additonal context

I suspect this is nothing to do with pulp-cli-deb and will happen equally for other content plugins, but I did not test this.

@quba42 quba42 added bug Something isn't working (template-set) Triage-Needed Needs to be reviewed at next pulp-cli mtg labels Mar 7, 2023
@ggainey
Copy link
Contributor

ggainey commented Mar 15, 2023

My hunch is that click-processing hits https://github.com/pulp/pulp-cli/blob/main/pulpcore/cli/common/generic.py#L356 before it's had a chance to add a RepositoryContext to the context, and we're asserting on "no repo found". To address this we prob want to do something like --version=name:version , --repository= implies latest_version, and you can't specify both.

At a minimum, we need to document that "order matters" in this case.

@ggainey ggainey removed the Triage-Needed Needs to be reviewed at next pulp-cli mtg label Mar 15, 2023
@MichalPysik MichalPysik self-assigned this Nov 17, 2023
@MichalPysik
Copy link
Member

I have tested the issue on a pulp file repository and can confirm that this is indeed not pulp-deb specific.

@gerrod3
Copy link
Contributor

gerrod3 commented Dec 5, 2023

This is the default behavior of click it seems: https://click.palletsprojects.com/en/8.1.x/advanced/#callback-evaluation-order. Going to need to be more clever in our callbacks if we want to be order invariant.

MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Dec 7, 2023
Use entered order of parameters matter, and can lead to unexpected behavior in some cases.
Resource lookup option is now an eager parameter, which means it is processed before all
non-eager parameters, no matter the order user entered them in.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Dec 11, 2023
Use entered order of parameters matter, and can lead to unexpected behavior in some cases.
Resource lookup option is now an eager parameter, which means it is processed before all
non-eager parameters, no matter the order user entered them in.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Dec 11, 2023
Use entered order of parameters matter, and can lead to unexpected behavior in some cases.
--repository is now an eager parameter, which means it is processed before all
non-eager parameters, no matter the order user entered them in. This applies to both
the option and the resource lookup option (but not on repository_href_option).

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Dec 15, 2023
Use entered order of parameters matter, and can lead to unexpected behavior in some cases.
--repository is now an eager parameter, which means it is processed before all
non-eager parameters, no matter the order user entered them in. This applies to both
the option and the resource lookup option (but not on repository_href_option).

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Jan 22, 2024
Use entered order of parameters matter, and can lead to unexpected behavior in some cases.
--repository is now an eager parameter, which means it is processed before all
non-eager parameters, no matter the order user entered them in. This applies to both
the option and the resource lookup option (but not on repository_href_option).

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 1, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 5, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 5, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 12, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 15, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 15, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 15, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 16, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 16, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 16, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 19, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 20, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 22, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 22, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 22, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
MichalPysik added a commit to MichalPysik/pulp-cli that referenced this issue Feb 22, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes pulp#650
mdellweg pushed a commit that referenced this issue Feb 22, 2024
The order in which the user enters options (--version, --repository, ..) no longer matters for
'pulp <plugin> repository version' commands, since all API calls are now deferred to after the
options' callbacks are processed.

closes #650
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working (template-set)
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants