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

Add global repository version list command #647

Merged
merged 1 commit into from Mar 15, 2023

Conversation

mdellweg
Copy link
Member

@mdellweg mdellweg commented Mar 6, 2023

fixes #631

Review Checklist:

  • An issue is properly linked. [feature and bugfix only]
  • Tests are present or not feasible.
  • Commits are split in a logical way (not historically).

Comment on lines 750 to 809
if self.repository_ctx.tangible:
return {self.repository_ctx.HREF: self.repository_ctx.pulp_href}
else:
return {}
Copy link
Member Author

Choose a reason for hiding this comment

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

Now i need to find a new way to make the existence of lookup values in the repository context mandatory for version detail commands. XD

Copy link
Contributor

Choose a reason for hiding this comment

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

When does scope get called? You could add more logic to the version generic command that checks list_only and sets a parameter on the context object which scope could check to make sure the lookup has been set.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, something like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

It sounds easier to not use the baseclass RepositoryContext for the generic endpoint, but subclass one for it. I will keep the tangible however. I like it.

@mdellweg mdellweg force-pushed the repo_ver_list branch 3 times, most recently from 47c8a1e to c637cb8 Compare March 9, 2023 11:58
@mdellweg mdellweg changed the title WIP Add global repository version list command Add global repository version list command Mar 13, 2023
@mdellweg mdellweg force-pushed the repo_ver_list branch 3 times, most recently from 66bdb2e to 2552089 Compare March 14, 2023 12:57
@mdellweg mdellweg marked this pull request as ready for review March 14, 2023 12:57
Comment on lines +154 to +156
if self.has_plugin(PluginRequirement("core", max="99.99.0")):
# https://github.com/pulp/pulpcore/issues/3634
for operation_id, (method, path) in self.api.operations.items():
Copy link
Contributor

Choose a reason for hiding this comment

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

You think we should look into moving this method into it's own file? context.py will grow quite large as we discover more schema bugs. Also, can plugins that are not apart of this package contribute to this method?

Copy link
Member Author

@mdellweg mdellweg Mar 15, 2023

Choose a reason for hiding this comment

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

No, they cannot, and that is a valid concern. Maybe we need some plugin lookup for glue parts after all.

) -> None:
result = repository_version_ctx.repair()
pulp_ctx.output_result(result)
callback.add_command(list_command(decorators=decorators + [content_in_option]))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also adding the --content fields to all of the other plugin repo version commands too, neat.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes!

@mdellweg mdellweg merged commit 74c60f2 into pulp:main Mar 15, 2023
11 checks passed
@mdellweg mdellweg deleted the repo_ver_list branch March 15, 2023 09:48
@mdellweg mdellweg added this to the 0.18.0 milestone Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to list repositories containing a content
2 participants