-
Notifications
You must be signed in to change notification settings - Fork 601
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
fix: misleading documentation on role name restrictions #2420
base: devel
Are you sure you want to change the base?
Conversation
Signed-off-by: gardar <[email protected]>
Thanks for your Ansible docs contribution! We talk about Ansible documentation on Matrix at #docs:ansible.im if you ever want to join us and chat about the docs! We meet on Matrix every Tuesday. See the Ansible calendar for meeting details. We welcome additions to our weekly agenda items too. You can add the |
@@ -168,7 +168,7 @@ roles directory | |||
|
|||
Collection roles are mostly the same as existing roles, but with a couple of limitations: | |||
|
|||
- Role names are now limited to contain only lowercase alphanumeric characters, plus ``_`` and start with an alpha character. | |||
- Role names can only contain lowercase alphanumeric characters and underscores ( ``_`` ). Role names prefixed with an underscore are not intended for direct user invocation and should be reserved for internal collection roles only. |
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 is still misleading, since it notes an external limitation imposed by Galaxy without the necessary level of detail to understand the limitation. Ansible itself only requires that the role name be a valid Python identifier.
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.
Good point, I'm assuming you are talking about this in particular:
The directory name of the role is used as the role name. Therefore, the directory name must comply with the above role name rules. The collection import into Galaxy will fail if a role name does not comply with these rules.
Do you want me to rephrase it or perhaps just remove it altogether?
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.
There are two things that should be documented. The first is that collections impose additional restrictions on role names, by requiring that they be valid Python identifiers [0]. The second is that Galaxy imposes further restrictions on role names, by enforcing a more limited character set.
[0] Actually, I appear to be confident but wrong here. I expected it to fail on a role named 2
but it works just fine, which leaves me with no idea of what the real restrictions are.
TASK [include_role : flowerysong.test.2] ***************************************
included: flowerysong.test.2 for localhost
TASK [flowerysong.test.2 : debug] **********************************************
ok: [localhost] =>
msg: Hello world!
TASK [include_role : flowerysong.test.한글] ************************************
included: flowerysong.test.한글 for localhost
TASK [flowerysong.test.한글 : debug] *******************************************
ok: [localhost] =>
msg: Hello world!
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.
According to https://github.com/ansible/ansible/blob/f11dfa7cce0f939a5dc1e11addc6cfbb5c7fe030/lib/ansible/utils/collection_loader/_collection_finder.py#L916 it's likely this:
VALID_FQCR_RE = re.compile(_to_text(r'^\w+(\.\w+){2,}$')) # can have 0-N included subdirs as well
According to https://docs.python.org/3/library/re.html, \w
covers:
Matches Unicode word characters; this includes all Unicode alphanumeric characters (as defined by str.isalnum()), as well as the underscore (
_
).
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.
I would separate this into several 'sub bullets', otherwise it is either too long or non substantive enough:
- In collections, role names are required to be valid Python identifiers <link to RE/PEP> ...
- If uploading to galaxy or Automation Hub, you are further restrticted to ...
- By convention, collection authors will prefix role names with an underscore to indicate they are for internal use and it is not advised for playbook authors to reference them directly.
@@ -168,7 +168,7 @@ roles directory | |||
|
|||
Collection roles are mostly the same as existing roles, but with a couple of limitations: | |||
|
|||
- Role names are now limited to contain only lowercase alphanumeric characters, plus ``_`` and start with an alpha character. | |||
- Role names can only contain lowercase alphanumeric characters and underscores ( ``_`` ). Role names prefixed with an underscore are not intended for direct user invocation and should be reserved for internal collection roles only. |
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.
- Role names can only contain lowercase alphanumeric characters and underscores ( ``_`` ). Role names prefixed with an underscore are not intended for direct user invocation and should be reserved for internal collection roles only. | |
- Role names can contain only lowercase alphanumeric characters and underscore (``_``) characters. Role names prefixed with an underscore are not intended for direct user invocation and should be reserved for internal collection 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.
There are also spaces around the underscore in the brackets which should be removed.
corrects the documentation to reflect that role names can begin with an underscore (_). It also recommends reserving such names for internal roles within a collection, not for direct user invocation.
fixes: #2035