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.

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

na_ontap_volume - added qos policy module options #56026

Merged
merged 5 commits into from May 21, 2019
Merged

na_ontap_volume - added qos policy module options #56026

merged 5 commits into from May 21, 2019

Conversation

vicmunoz
Copy link
Contributor

@vicmunoz vicmunoz commented May 2, 2019

SUMMARY

Added new options:
-- qos_policy_group
-- qos_adaptive_policy_group

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

na_ontap_volume

@ansibot
Copy link
Contributor

ansibot commented May 2, 2019

cc @amit0701 @carchi8py @hulquest @lmprice @lonico @ndswartz @schmots1
click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 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. netapp new_contributor This PR is the first contribution by a new community member. storage support:certified This issue/PR relates to certified code. labels May 2, 2019
@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label May 3, 2019
carchi8py
carchi8py suggested changes May 3, 2019
View reviewed changes
Copy link
Contributor

@carchi8py carchi8py left a comment

Choose a reason for hiding this comment

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

Can we limit the change to just adding the qos policy necessary changes?

@vicmunoz
Copy link
Contributor Author

vicmunoz commented May 6, 2019

Other changes added are just pylint recommendations or similar. Is it ok for you or prefer to remove them?

@carchi8py
Copy link
Contributor

Which one are pylint recommendations?

and are you running the standard pylint or the modify one ansible uses
ansible-test sanity --docker=default --test pylint

@vicmunoz
Copy link
Contributor Author

vicmunoz commented May 7, 2019

alignments, lines too long...
I've executed pylint standard. I didn't know ansible pylint.
I'll review and check using ansible sanity test from now on

lonico
lonico approved these changes May 7, 2019
View reviewed changes
Copy link
Contributor

@lonico lonico left a comment

Choose a reason for hiding this comment

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

shipit

lib/ansible/modules/storage/netapp/na_ontap_volume.py Outdated Show resolved Hide resolved
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label May 8, 2019
thedoubl3j
thedoubl3j requested changes May 10, 2019
View reviewed changes
Copy link
Member

@thedoubl3j thedoubl3j left a comment

Choose a reason for hiding this comment

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

if we could get a couple of examples of the new features added in, that would be great

@vicmunoz
Copy link
Contributor Author

updated details and examples

@thedoubl3j
Copy link
Member

@carchi8py @lonico please re ship it when you get a moment to review.

lonico
lonico approved these changes May 15, 2019
View reviewed changes
Copy link
Contributor

@lonico lonico left a comment

Choose a reason for hiding this comment

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

shipit

lib/ansible/modules/storage/netapp/na_ontap_volume.py Show resolved Hide resolved
lonico
lonico approved these changes May 17, 2019
View reviewed changes
Copy link
Contributor

@lonico lonico left a comment

Choose a reason for hiding this comment

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

Shipit

thedoubl3j
thedoubl3j approved these changes May 17, 2019
View reviewed changes
Copy link
Member

@thedoubl3j thedoubl3j left a comment

Choose a reason for hiding this comment

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

examples added for both, lgtm.

@thedoubl3j
Copy link
Member

shipit

@thedoubl3j
Copy link
Member

confirmed by vendor maintainers as good to go. @lonico @carchi8py merging

@thedoubl3j thedoubl3j merged commit a608ca0 into ansible:devel May 21, 2019
@vicmunoz vicmunoz deleted the volume_qos branch May 23, 2019 10:43
@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@carchi8py carchi8py carchi8py requested changes

@lonico lonico lonico approved these changes

@thedoubl3j thedoubl3j thedoubl3j approved these changes

Assignees
No one assigned
Labels
affects_2.9 This issue/PR affects Ansible v2.9 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. netapp storage support:certified This issue/PR relates to certified code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@vicmunoz @ansibot @carchi8py @thedoubl3j @lonico