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

ec2_eni: change data type of device_index to str when passing it to api as expected by api call #877

Merged

Conversation

mandar242
Copy link
Contributor

@mandar242 mandar242 commented Jun 17, 2022

SUMMARY

Currently the data type for parameter device_index here while being passed to api call is integer but one of the API calls later used here in the module (describe_network_interfaces) expects it to be string as per boto3 api documentation.

Fixes #870

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_eni

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review module module needs_triage plugins plugin (any type) small_patch Hopefully easy to review labels Jun 17, 2022
@mandar242 mandar242 changed the title ec2_eni: change data type of device_index to str as expected by api call ec2_eni: change data type of device_index to str when passing it to api as expected by api call Jun 17, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 58s
✔️ build-ansible-collection SUCCESS in 4m 43s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 34s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 27s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 57s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 52s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 01s
✔️ ansible-test-splitter SUCCESS in 2m 20s
✔️ integration-amazon.aws-1 SUCCESS in 13m 37s
⚠️ integration-amazon.aws-2 SKIPPED
⚠️ integration-amazon.aws-3 SKIPPED
⚠️ integration-amazon.aws-4 SKIPPED
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-community.aws-1 SKIPPED
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

It would be really good if we can get an integration test in place to catch this. I thought we did and I'm slightly surprised it didn't fail.

Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

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

code change lgtm once the PR has test coverage

@ansibullbot ansibullbot added integration tests/integration tests tests and removed small_patch Hopefully easy to review labels Jun 17, 2022
@mandar242 mandar242 requested review from jillr and tremble June 17, 2022 23:09
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 24s
✔️ build-ansible-collection SUCCESS in 5m 15s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 42s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 10m 22s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 10m 39s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 08s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 22s
✔️ ansible-test-splitter SUCCESS in 2m 53s
✔️ integration-amazon.aws-1 SUCCESS in 15m 04s
⚠️ integration-amazon.aws-2 SKIPPED
⚠️ integration-amazon.aws-3 SKIPPED
⚠️ integration-amazon.aws-4 SKIPPED
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
✔️ integration-community.aws-1 SUCCESS in 6m 43s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Jun 22, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 08s
✔️ build-ansible-collection SUCCESS in 5m 05s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 8m 27s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 43s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 10m 05s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 7m 18s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 01s
✔️ ansible-test-splitter SUCCESS in 2m 27s
✔️ integration-amazon.aws-1 SUCCESS in 15m 08s
⚠️ integration-amazon.aws-2 SKIPPED
⚠️ integration-amazon.aws-3 SKIPPED
⚠️ integration-amazon.aws-4 SKIPPED
⚠️ integration-amazon.aws-5 SKIPPED
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
✔️ integration-community.aws-1 SUCCESS in 5m 35s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit f0fa859 into ansible-collections:main Jun 22, 2022
@github-actions
Copy link

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@mandar242 mandar242 deleted the 870-ec2_eni branch June 22, 2022 16:16
jatorcasso pushed a commit to jatorcasso/amazon.aws that referenced this pull request Jun 27, 2022
… api as expected by api call (ansible-collections#877)

ec2_eni: change data type of `device_index` to str when passing it to api as expected by api call

SUMMARY

Currently the data type for parameter device_index here while being passed to api call is integer but one of the API calls later used here in the module (describe_network_interfaces) expects it to be string as per boto3 api documentation.
Fixes ansible-collections#870

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

ec2_eni

Reviewed-by: Mark Chappell <None>
Reviewed-by: Jill R <None>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
cloudfront_distribution: add missing documentation

SUMMARY
Closes ansible-collections#877
The modul resprects this parameter already.
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
cloudfront_distribution

Reviewed-by: Mark Woolley <mw@marknet15.com>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
cloudfront_distribution: add missing documentation

SUMMARY
Closes ansible-collections#877
The modul resprects this parameter already.
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
cloudfront_distribution

Reviewed-by: Mark Woolley <mw@marknet15.com>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
cloudfront_distribution: add missing documentation

SUMMARY
Closes ansible-collections#877
The modul resprects this parameter already.
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
cloudfront_distribution

Reviewed-by: Mark Woolley <mw@marknet15.com>
Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review integration tests/integration mergeit Merge the PR (SoftwareFactory) module module plugins plugin (any type) tests tests
Projects
None yet
5 participants