-
Notifications
You must be signed in to change notification settings - Fork 83
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
Fixes #38019 - Add hammer support for flatpak remotes and remote repositories #971
Conversation
9d23163
to
d560b36
Compare
Test failures are due to rubocop autocorrect on my last push..Will push.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems there are missing organization options for deleting remotes:
Usage:
hammer flatpak-remote <delete|destroy> [OPTIONS]
Options:
--id NUMBER Flatpak remote id
--name VALUE Flatpak remote name to search by
-h, --help Print help
[vagrant@centos7-hammer-devel hammer-cli-katello]$ hammer flatpak-remote delete --name "Fedora flatpak"
Could not delete the Flatpak Remote.:
Error: Missing options to search organization.
[vagrant@centos7-hammer-devel hammer-cli-katello]$ hammer flatpak-remote delete --name "Fedora flatpak" --organization-id 1
Could not delete the Flatpak Remote.:
Error: Unrecognised option '--organization-id'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help text for creating remotes is a little confusing, specifically for the URLs:
Usage:
hammer flatpak-remote create [OPTIONS]
Options:
--description VALUE Description of the flatpak remote
--name VALUE Name
--organization[-id|-title|-label] VALUE/NUMBER Organization name/title/label/id
--registry-url VALUE Optional Registry url. Will be updated when remote is scanned.
--token VALUE Token/password for the flatpak remote
--url VALUE Url
--username VALUE Username for the flatpak remote
-h, --help Print help
It isn't quite clear what the difference is between the url
and the registry-url
. It also isn't clear why someone would need to enter a registry-url
.
Besides my comments above, the general workflows seem to be working well 👍 |
34bf5cc
to
980bb2b
Compare
Updated to address reviews. 👍🏼 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more comments:
end | ||
end | ||
|
||
class MirrorCommand < HammerCLIKatello::SingleResourceCommand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps unrelated to this PR, but do you know what "organization title" is?
We typically only have ID / Name / Label...
Usage:
hammer flatpak-remote remote-repository mirror [OPTIONS]
Options:
--async Do not wait for the task
--flatpak-remote[-id] VALUE/NUMBER Name/Id of associated flatpak remote
--id NUMBER Flatpak remote repository id
--name VALUE Name to search by
--organization[-id|-title|-label] VALUE/NUMBER Name/Title/Label/Id of associated organization
--product[-id] VALUE/NUMBER Product name/id to mirror the remote repository to
-h, --help Print help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[2] pry(main)> org.title
=> "Default Organization"
[3] pry(main)> org.name
=> "Default Organization"
[4] pry(main)> org.label
=> "Default_Organization"
Title seems to be the same as name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting!
0cf5a73
to
2c53124
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Merging.. 🎉 |
Adds hammer support for flatpak remotes. The sub-commands added are list/show/create/update/delete and scan.
This also supports list/info on remote repository and adds a command to mirror remote repository into katello.
This requires Katello/katello#11223 for testing..
Working on more tests but the commands should be ready for testing.