-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Conversation
Thanks @jmenga for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
ready_for_review |
Thanks @jmenga for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion. [This message brought to you by your friendly Ansibull-bot.] |
@mixja I don't have ~/.boto set up, only AWS_REGION set in my environment.
I had to add the region to the client call
I saw a commit with these details was removed. I noticed these modules were set up the same way
though I haven't tried using them. $ ansible --version |
@viper233 - as per Boto3 docs you can use AWS_DEFAULT_REGION (AWS_REGION is not listed as a support variable) to set the region. I will update the documentation of the module to reflect this needs to be set. EDIT: updated incorrect Boto3 link above |
I pass the region in as a parameter.
|
@viper233 - OK I see the problem, I will revise the module to support this style of configuration |
Is this also the case for the other modules too? I can try and test them later today and submit a PR |
@viper233 - sorry not sure which other modules you are referring to. I have added support for specifying AWS connection/region parameters explicitly. |
I did a grep on the modules directories to see if there were any other modules using the boto3.client call, these were the others that I came up with.
I haven't tested these modules yet. It's something I should do in my spare time. |
shipit |
1 similar comment
shipit |
@gregdek - can you reattempt the Travis CI build as from the logs it looks like a build issue unrelated to the module code. |
@jmenga this is likely due to the age of the PR; please rebase against the latest devel. When you've done so, ping me and we'll put this module into the new module review meeting, since it has the necessary shipits. |
acc6067
to
d6c8e81
Compare
Thanks @gregdek - rebased the PR and all checks pass |
description: | ||
- Gets information about an AWS CloudFormation stack | ||
dependencies: | ||
- boto3>=1.0.0 |
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.
This documentation field needs to be requirements, not dependencies.
I think that boto3 requires python >= 2.6? If so, need to mention python >= 2.6 as a requirement as well.
|
||
# import module snippets | ||
from ansible.module_utils.basic import * | ||
from ansible.module_utils.ec2 import * |
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.
Note -- we aren't requiring it yet but you don't have to treat these specially anymore. They can go at the top of the file with the other imports and you can import the specific things you need instead of doing a wildcard import.
needs_revision |
Thanks @jmenga for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
ready_for_review |
response = self.client.get_template(StackName=stack_name) | ||
return response.get('TemplateBody') | ||
except Exception as e: | ||
self.module.fail_json(msg="Error getting stack template - " + str(e)) |
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.
For all these fails, can you include the traceback like so:
try:
# some stuff
except Exception as e:
self.module.fail_json(msg="Error getting stack template - " + str(e), exception=traceback.format_exc(e))
And import traceback
at the top of the file?
Other than the exception handling, this looks good. |
ready_for_review |
Thanks @jmenga for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion. [This message brought to you by your friendly Ansibull-bot.] |
Looks great - I'm going to commit a fix to one of the boto3 connection creation manually. There is a problem with using this module with profiles. |
ISSUE TYPE
COMPONENT NAME
cloudformation_facts
ANSIBLE VERSION
SUMMARY
Provides facts about AWS CloudFormation stacks. Note there is another open pull request #1230 however my module has much more functionality.
In its most basic form, it will return stack description and also provides some useful transformations (
stack_outputs
andstack_parameters
in the example below) to make stack outputs and stack parameters easier to work with:A number of optional parameters can be specified to include the following additional stack information:
Example Usage:
Example expanded outputs: