-
Notifications
You must be signed in to change notification settings - Fork 328
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
ec2_instance_info - add support for include_attributes #1577
ec2_instance_info - add support for include_attributes #1577
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
027d7a5
to
0ca75a2
Compare
recheck |
0ca75a2
to
30d1e0f
Compare
ec2_instance_info: | ||
filters: | ||
"tag:Name": "{{ ec2_instance_name }}" | ||
include_attributes: |
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.
I think we should include all the attribute choices in the test to verify that they all work as expected (I'm seeing that the boto3 docs include enaSupport
as an option and also say Note: The enaSupport attribute is not supported at this time.
which is...confusing).
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 is like you want to test the AWS API, this is not the scope of ansible.
I can removed enaSupport
for now as it is not supported now
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.
Not really. It's ensuring that the choices we include in our ansible module parameters are correct and are passed correctly to boto3. I have already seen examples in other modules where we assume that this is the case and it is in fact not, but we didn't realize it because we don't test every option in the spec.
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.
I will remove the choices and use the module the same way we are doing for our info modules as this may changes with new values with new boto3 release
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.
@hakbailey I have update the tests as required, hope it is ok now
recheck |
…rt yet (boto3 1.28.2)
bd74247
to
554318f
Compare
regate |
ab9b074
into
ansible-collections:main
SUMMARY
include_attributes
allows the module to describe specific attributes for an EC2 instanceISSUE TYPE
COMPONENT NAME
ec2_instance_info