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

Jump to bottom

Provide a factory for resource options #161

Merged
merged 2 commits into from May 26, 2021
Merged

Provide a factory for resource options #161

merged 2 commits into from May 26, 2021

Conversation

mdellweg
Copy link
Member

The options accept resources by the "[[:]:]name" pattern.

fixes #158

@mdellweg
Copy link
Member Author

WIP: should be used in more places if the design is agreed upon.

@gerrod3
Copy link
Contributor

gerrod3 commented Feb 25, 2021

Should we limit the naming convention to only be used for options where a known resource, i.e. remotes, repositories, content, ..., is being asked for? Basically, should we have a global way to refer to a resource besides that resource's pulp_href? Something like "pulp show --name repository:file:file:foo". Maybe it doesn't make sense since there isn't many other operations we could do across all resources that aren't covered by their specific plugin commands already, but maybe you have some other ideas.

Other than that I think the design looks good!

daviddavis
daviddavis reviewed Mar 6, 2021
View reviewed changes
pulpcore/cli/common/generic.py Outdated

def resource_option(*args: Any, **kwargs: Any) -> Callable[[F], F]:
default_plugin: str = kwargs.pop("default_plugin", None)
default_type: str = kwargs.pop("default_type", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@daviddavis
Copy link
Contributor

This looks good to me so far.

@mdellweg mdellweg force-pushed the generic_resource_option branch 2 times, most recently from 2ff31d9 to bd92fc7 Compare March 19, 2021 18:40
@mdellweg mdellweg force-pushed the generic_resource_option branch 2 times, most recently from 25bb4f0 to e009e37 Compare March 27, 2021 10:20
@mdellweg mdellweg force-pushed the generic_resource_option branch 2 times, most recently from cdc343d to 1d59e2a Compare April 10, 2021 17:44
@mdellweg mdellweg force-pushed the generic_resource_option branch 2 times, most recently from 7dd5c40 to e8ce1ad Compare April 28, 2021 14:59
@mdellweg mdellweg force-pushed the generic_resource_option branch 4 times, most recently from 4fb0606 to d748f73 Compare May 9, 2021 15:56
@mdellweg mdellweg marked this pull request as ready for review May 9, 2021 16:03
@mdellweg mdellweg force-pushed the generic_resource_option branch 2 times, most recently from a991279 to e45ba89 Compare May 9, 2021 16:28
@daviddavis
Copy link
Contributor

The options accept resources by the "[[:]:]name" pattern.

Can you add test(s) for this?

@daviddavis
Copy link
Contributor

Also this needs a rebase. :)

@mdellweg mdellweg force-pushed the generic_resource_option branch 4 times, most recently from e2953e5 to 8e760e1 Compare May 12, 2021 18:07
@mdellweg
Copy link
Member Author

The options accept resources by the "[[:]:]name" pattern.

Can you add test(s) for this?

Tests are added.
You can now specify a remote of the name test_1 as:

  • test_1
  • :test_1
  • ::test_1
  • file::test_1
  • file:file:test_1
  • :file:test_1
    and test:2 as:
  • ::test:2
  • file::test:2
  • file:file:test:2
  • :file:test:2

Obviously we discourage the use of : in names, but we can still handle it.

@mdellweg mdellweg force-pushed the generic_resource_option branch 2 times, most recently from 028785b to 14b1e11 Compare May 22, 2021 19:41
daviddavis
daviddavis approved these changes May 26, 2021
View reviewed changes
Copy link
Contributor

@daviddavis daviddavis left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mdellweg
Remove deprecated commands
7ead17b 
[noissue]
@mdellweg
Provide a factory for resource options
4565922 
The options accept resources by the "[[<plugin>:]<type>:]name" pattern.

Fixes pulp#158
@mdellweg mdellweg merged commit 42f6232 into pulp:develop May 26, 2021
@mdellweg mdellweg deleted the generic_resource_option branch May 26, 2021 16:11
@mdellweg mdellweg added this to the 0.10.0 milestone Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daviddavis daviddavis daviddavis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.10.0
Development

Successfully merging this pull request may close these issues.

Allow to specify plugin and type along with the name for a dependent resource
3 participants
@mdellweg @gerrod3 @daviddavis