-
Notifications
You must be signed in to change notification settings - Fork 23.9k
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
Clarify galaxy CLI --help about install locations #81159
Conversation
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 install subcommand isn't listed in ansible-galaxy --help
I agree this would be good to mention since it supports collections in addition to roles. Maybe the description could be amended to generally mention the role subcommands:
Perform various Role and Collection related operations. The role subcommands have top-level aliases.
ansible-galaxy install
has additional support to install collections from a requirements file if--roles-path
is not provided.
@@ -277,7 +277,7 @@ def init_parser(self): | |||
collections_path = opt_help.ArgumentParser(add_help=False) | |||
collections_path.add_argument('-p', '--collections-path', dest='collections_path', type=opt_help.unfrack_path(pathsep=True), | |||
action=opt_help.PrependListAction, | |||
help="One or more directories to search for collections in addition " | |||
help="One or more directories to search for collections (or install to) in addition " |
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.
This collections_path option is not the one used by install (https://github.com/ansible/ansible/blob/devel/lib/ansible/cli/galaxy.py#L512-L514), and can include paths ansible-galaxy collection install will not use.
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.
Correct that, in devel
branch, this --collections-path
option is not used by ansible-galaxy install
, and I'm not really trying to change that. For my own selfish purposes, I just want to consolidate our use cases such that we use the environment variables ANSIBLE_COLLECTIONS_PATHS and ANSIBLE_ROLES_PATH. And the reason I made this PR is because I found that non-obvious.
You can use the --collections-path
in this install command, which is why I made this particular change to this line:
ansible-galaxy collection install awx.awx -p alan
My gripe was that this help text was written assuming the user was running ansible-galaxy collection list -p alan/
, but the option was reused for the ansible-galaxy collection install
.
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 --collections-path
option is added in two locations and has different help texts (and different types and defaults). Changing the --help for the list subcommand here doesn't affect the option in ansible-galaxy collection install --help
.
if self._implicit_role: | ||
# installing both roles and collections | ||
description_text = ( | ||
'Install roles and collections from file(s), URL(s) or Ansible ' | ||
'Galaxy to the first entry in the config COLLECTIONS_PATH for collections ' | ||
'and first entry in the config ROLES_PATH for roles.' | ||
) |
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.
This could mention that if -p
/--roles-path
is provided only roles are installed. If there are collections in the requirements.yml too, a warning is given about the potentially ambiguous option, and that collections won't be installed.
the ansible-galaxy install command has a --roles-path CLI option, but no collections path option
None of the collection-specific CLI install options are supported, only collection options that are also supported by roles. Role-specific options are supported for backwards compatibility because ansible-galaxy install
handled roles originally.
If a --collections-path
option is added, -p
should probably be removed as a valid alias for --roles-path
, since it's also an alias normally for --collections-path
.
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.
tempted to say just use --install-path
and even if people see the 'wrong' help .. it will be right!
We can put it before existing options, move those to last and add deprecation to them in future
Co-authored-by: Sandra McCann <[email protected]>
consider as a 2.18 task |
@AlanCoding I opened a PR with your modification and the requested changes, if you have an opportunity to take a look I'd appreciate your feedback #83919. I removed your change here since the
|
* add descriptions for `ansible-galaxy install` and `ansible-galaxy role|collection install` * fix the usage for installing roles and collections together and include collections in the description for -r Closes ansible#81159 Co-authored-by: Alan Rominger <[email protected]> Co-authored-by: Sandra McCann <[email protected]> (cherry picked from commit 85d9a40)
* add descriptions for `ansible-galaxy install` and `ansible-galaxy role|collection install` * fix the usage for installing roles and collections together and include collections in the description for -r Closes ansible#81159 Co-authored-by: Alan Rominger <[email protected]> Co-authored-by: Sandra McCann <[email protected]> (cherry picked from commit 85d9a40)
…3964) * Clarify galaxy CLI --help about install locations (#83919) * add descriptions for `ansible-galaxy install` and `ansible-galaxy role|collection install` * fix the usage for installing roles and collections together and include collections in the description for -r Closes #81159 Co-authored-by: Alan Rominger <[email protected]> Co-authored-by: Sandra McCann <[email protected]> (cherry picked from commit 85d9a40) * ansible-galaxy - fix the usage for role/collection install (#83979) (cherry picked from commit bf8da52)
…3963) * Clarify galaxy CLI --help about install locations (#83919) * add descriptions for `ansible-galaxy install` and `ansible-galaxy role|collection install` * fix the usage for installing roles and collections together and include collections in the description for -r Closes #81159 Co-authored-by: Alan Rominger <[email protected]> Co-authored-by: Sandra McCann <[email protected]> (cherry picked from commit 85d9a40) * ansible-galaxy - fix the usage for role/collection install (#83979) (cherry picked from commit bf8da52)
SUMMARY
I asked a lot of questions to @john-westcott-iv on ansible/awx#14081 which we discovered really came down to docs of the
ansible-galaxy
commands themselves.In #67843 a new
ansible-galaxy install
command was introduced which could install both roles and collections. The options are well-documented, but the effect of the command is actually unclear. To explain why this the case, consider:What does this command do? The help text should show this in the main body of the text. It doesn't quite do that, but it does have that information buried in an option.
Yes, it is obvious that it installs roles but where does it install them? This help text answers that.
Surprisingly, this question is not answered at all for either
ansible-galaxy install --help
oransible-galaxy collection install --help
ISSUE TYPE
COMPONENT NAME
lib/ansible/cli/galaxy.py
ADDITIONAL INFORMATION
With this change, we get:
and
To me, this answers the question of "what does this do?" for each command.
There are still some other shortcomings that I did not address with this, but I want to mention anyway:
ansible-galaxy install
command has a--roles-path
CLI option, but no collections path optioninstall
subcommand isn't listed inansible-galaxy --help
For now, I wanted to stop at a purely docs patch, as this should change the help text rendering, but not change anything about the behavior.