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.

Sign up for GitHub

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

Jump to bottom

Add non_unique parameter for group module #53085

Merged
merged 1 commit into from Mar 13, 2019
Merged

Add non_unique parameter for group module #53085

merged 1 commit into from Mar 13, 2019

Conversation

gabriel376
Copy link
Contributor

@gabriel376 gabriel376 commented Feb 27, 2019 •
edited

Add non_unique parameter for group module
closes #30233

SUMMARY
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • group
ADDITIONAL INFORMATION
  • I used module user as reference. I could not find HPUX on group module that exists on user module
  • I could not find tests for non_unique parameter on user module
  • The option non_unique is not available for darwin/macOs systems

@ansibot
Copy link
Contributor

ansibot commented Feb 27, 2019

cc @sfromm
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. system System category core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 27, 2019
samdoran
samdoran suggested changes Mar 1, 2019
View reviewed changes
lib/ansible/modules/system/group.py Outdated Show resolved Hide resolved
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. labels Mar 1, 2019
samdoran
samdoran suggested changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@samdoran samdoran left a comment

Choose a reason for hiding this comment

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

Could you add a check making gid a requirement of non_unique? I wasn't able to find a good way to do that with our built in checking methods.

lib/ansible/modules/system/group.py Outdated Show resolved Hide resolved
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 11, 2019
@gabriel376
Copy link
Contributor Author

Could you add a check making gid a requirement of non_unique? I wasn't able to find a good way to do that with our built in checking methods.

Can you give more details or provide an example ?

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 12, 2019
@samdoran
Copy link
Contributor

samdoran commented Mar 12, 2019 •
edited

Actually, after looking through our validation methods again, it looks like required_by will do the trick:

required_by={'non_unique': 'gid'}

@gabriel376
Copy link
Contributor Author

Actually, after looking through our validation methods again, it looks like required_by will do the trick:

required_by={'non_unique': 'gid'}

Done!
Thank you!

@samdoran
Copy link
Contributor

The reason required_by didn't work is because it expects the key to be missing or it to have a value of None. Setting the default of non_unique to False broke the logic in required_by.

Your solution with required_if is just fine, though. Sorry I didn't give you better guidance but I'm glad you figured it out!

@samdoran samdoran merged commit f97a593 into ansible:devel Mar 13, 2019
@gabriel376
Copy link
Contributor Author

You helped a lot!
Thank you!

@ansible ansible locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@samdoran samdoran samdoran approved these changes

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. system System category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

non_unique parameter for group module
3 participants
@gabriel376 @ansibot @samdoran