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

New module azure_rm_managementgroup_info #428

Conversation

paultaiton
Copy link
Contributor

SUMMARY

info module for management groups.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

azure_rm_managementgroup_info

ADDITIONAL INFORMATION

This is a read only info module for querying Azure RM and gather details about management groups.

I have added integration test tasks, but I'm not sure what kind of role assignments exist for the test accounts, and can't be 100% certain they will work. It requires at least 1 management group exists which the principal has reader access to.

Copy link
Collaborator

@Fred-sun Fred-sun left a comment

Choose a reason for hiding this comment

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

Please add follow lines to ~/tests/sanity/ignore-2.10.txt and ~/tests/sanity/ignore-2.11.txt

plugins/modules/azure_rm_managementgroup_info.py validate-modules:required_if-requirements-unknown
plugins/modules/azure_rm_managementgroup_info.py validate-modules:required_if-unknown-key

Copy link
Collaborator

@Fred-sun Fred-sun left a comment

Choose a reason for hiding this comment

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

please remove the ANSIBLE_METADATA block, it is no longer needed.

Copy link
Collaborator

@Fred-sun Fred-sun left a comment

Choose a reason for hiding this comment

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

Please add "disabled" to tests/integration/targets/azure_rm_managementgroup/aliases, Thank you very mcuh!

@paultaiton
Copy link
Contributor Author

Hello,

Your suggestions have been committed. Thank you for the review @Fred-sun

Copy link
Collaborator

@Fred-sun Fred-sun left a comment

Choose a reason for hiding this comment

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

@paultaiton It's my mistake! Please add follow lines to ~/tests/sanity/ignore-2.10.txt

plugins/modules/azure_rm_managementgroup_info.py validate-modules:required_if-requirements-unknown
plugins/modules/azure_rm_managementgroup_info.py validate-modules:required_if-unknown-key

@paultaiton
Copy link
Contributor Author

@paultaiton It's my mistake! Please add follow lines to ~/tests/sanity/ignore-2.10.txt

haha, I was wondering. No problem sir, we're all on the same team.

@Fred-sun
Copy link
Collaborator

Fred-sun commented Mar 9, 2021

@paultaiton There should be no problem. We will complete the review as soon as possible and recommend the merger! Thank you very much!

@Fred-sun Fred-sun added the ready_for_review The PR has been modified and can be reviewed and merged label Mar 9, 2021
@paultaiton
Copy link
Contributor Author

@paultaiton There should be no problem. We will complete the review as soon as possible and recommend the merger! Thank you very much!

Awesome. Thank you very much @Fred-sun

Copy link
Collaborator

@Fred-sun Fred-sun left a comment

Choose a reason for hiding this comment

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

Follow other module's define!

@paultaiton
Copy link
Contributor Author

@Fred-sun
I had to update a couple lines in addition to your suggestion to make it work, but they are now pushed into the branch.

@haiyuazhang haiyuazhang merged commit 1b56ba4 into ansible-collections:dev Mar 17, 2021
@paultaiton
Copy link
Contributor Author

Thanks @haiyuazhang , appreciate it.

@paultaiton paultaiton deleted the paultaiton_20210121_managementgroup-info branch March 17, 2021 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants