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: digital_ocean_region_facts #35911
Conversation
@pmarques Could you please review this ? |
LGTM Tested module with Ansible version 2.6 |
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.
Some comments inline
author: "Abhijeet Kasurde (@akasurde)" | ||
version_added: "2.5" | ||
options: | ||
oauth_token: |
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.
Use documentation fragment
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.
OK.
description: DigitalOcean regions facts | ||
returned: success | ||
type: list | ||
sample: [ |
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.
change to data
# Check if api_token is valid or not | ||
response = rest.get('account') | ||
if response.status_code == 401: | ||
module.fail_json(msg='Failed to login using API token, please verify validity of API token.') |
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.
Remove this because is already inside DO utils
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.
ok.
break | ||
page += 1 | ||
regions.extend(response.json["regions"]) | ||
has_next = "pages" in response.json["links"] and "next" in response.json["links"]["pages"] |
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.
If PR #36004 is merged, can we update this to use get_paginated_data?
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.
Yes. This is old module. And 36004 got merged before this. I will update rest of them to use pagination.
def main(): | ||
module = AnsibleModule( | ||
argument_spec=dict( | ||
oauth_token=dict(aliases=['DO_API_TOKEN', 'DO_API_KEY', 'DO_OAUTH_TOKEN'], |
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.
Use digital_ocean_argument_spec form DO utils.
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.
OK.
0538bfc
to
0a23a2a
Compare
0a23a2a
to
1fe366d
Compare
@BondAnthony @Akasurde @alukovenko @harneksidhu @kontrafiktion @mgregson @pmarques @zbal As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
LTGM Thanks @Akasurde |
Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
1fe366d
to
8152825
Compare
@BondAnthony @pmarques Thanks for review. |
SUMMARY
Signed-off-by: Abhijeet Kasurde akasurde@redhat.com
ISSUE TYPE
COMPONENT NAME
lib/ansible/modules/cloud/digital_ocean/digital_ocean_region_facts.py
ANSIBLE VERSION