-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat(rpm_distribution): add pulp_labels parameter #175
feat(rpm_distribution): add pulp_labels parameter #175
Conversation
This changeset adds `pulp_labels` as an optional parameter to the `rpm_distribution` module. It also adds tests and fixtures.
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.
Thank you for that addition.
I do have one improvement in mind though.
@@ -112,6 +127,7 @@ def main(): | |||
"repository": {}, | |||
"content_guard": {}, | |||
"generate_repo_config": {"type": "bool"}, | |||
"pulp_labels": {"type": "dict"}, |
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's a way to specify this as a dictionary of strings. Let's do that.
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.
Hey @mdellweg. I've looked into this and cannot determine the method you describe. Specifically, it doesn't appear that argument_spec
allows for arbitrary keys, which pulp_labels
does. I could potentially change pulp_labels
to a list of dictionaries, but that seems generally unhelpful to me. Could you provide some additional guidance if you know of a better way?
Here is some discussion I found, for reference.
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.
Have you tried "elements" or does that only work with "list"? I may have been wrong.
For reference: https://docs.ansible.com/ansible/latest/dev_guide/developing_program_flow_modules.html#argument-spec
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.
Unfortunately, that is only for use with the list
type. I did give it a try just for fun, but it seems documentation does in fact match implementation on this one.
"msg": "Invalid type dict for option '{'key1': 'value1', 'key2': 'value2'}', elements value check is supported only with 'list' type"
This change adds
pulp_labels
as an optional parameter to therpm_distribution
module. It also adds tests and fixtures.