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_vol: Add support for OutpostArn param #597

Merged

Conversation

mandar242
Copy link
Contributor

SUMMARY

Added support for OutpostArn param in ec2_vol.

Fixes #366.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ec2_vol

@alinabuzachis
Copy link
Collaborator

@mandar242 Thank you for working on this. The main issue with AWS Outpost is that we cannot test it at the moment. I would suggest to kindly ask @hferreira23 for testing this feature if possible.

@hferreira23
Copy link

I'm on vacation right now :) Once I go back to the office (where I have access to an Outpost), I'll try this patch.

@hferreira23
Copy link

@alinabuzachis @mandar242, sorry that took me a while but from what I could test it seems to work as we hope. I created the volume in the Outpost just fine.

@alinabuzachis
Copy link
Collaborator

@alinabuzachis @mandar242, sorry that took me a while but from what I could test it seems to work as we hope. I created the volume in the Outpost just fine.

No problem! Thank you for testing it!

plugins/modules/ec2_vol.py Outdated Show resolved Hide resolved
plugins/modules/ec2_vol.py Show resolved Hide resolved
goneri
goneri previously requested changes Jan 12, 2022
tests/unit/requirements.txt Outdated Show resolved Hide resolved
test-requirements.txt Outdated Show resolved Hide resolved
tests/unit/module_utils/test_ec2.py Outdated Show resolved Hide resolved
# ec2.is_outposts_arn
# ========================================================

@parametrize(
Copy link
Member

Choose a reason for hiding this comment

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

Use @pytest.mark.parametrize instead.

Copy link
Collaborator

@alinabuzachis alinabuzachis Jan 12, 2022

Choose a reason for hiding this comment

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

@goneri @pytest.mark.parametrize cannot be used with unittest.TestCase methods, for this reason Mandar used parametrize. He is trying to fix this by placing the test method outside of the class.

@softwarefactory-project-zuul
Copy link
Contributor

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

@alinabuzachis
Copy link
Collaborator

Some pep8 errors to be addressed. @mandar242. Otherwise, this LGTM!

2022-01-13 13:48:09.025274 \| fedora-35 \| ERROR: Found 5 pep8 issue(s) which need to be resolved:
2653 | 2022-01-13 13:48:09.025533 \| fedora-35 \| ERROR: plugins/modules/ec2_vol.py:519:13: E303: too many blank lines (2)
2654 | 2022-01-13 13:48:09.025663 \| fedora-35 \| ERROR: tests/unit/module_utils/test_ec2.py:86:1: E305: expected 2 blank lines after class or function definition, found 1
2655 | 2022-01-13 13:48:09.025777 \| fedora-35 \| ERROR: tests/unit/module_utils/test_ec2.py:87:9: E126: continuation line over-indented for hanging indent
2656 | 2022-01-13 13:48:09.025868 \| fedora-35 \| ERROR: tests/unit/module_utils/test_ec2.py:92:9: E123: closing bracket does not match indentation of opening bracket's line
2657 | 2022-01-13 13:48:09.025971 \| fedora-35 \| ERROR: tests/unit/module_utils/test_ec2.py:94:1: E302: expected 2 blank lines, found 1

@jillr jillr added the gate label Jan 24, 2022
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

@alinabuzachis alinabuzachis dismissed goneri’s stale review January 25, 2022 18:11

Two approvals already!

@alinabuzachis alinabuzachis added gate and removed gate labels Jan 25, 2022
@ansible-zuul ansible-zuul bot merged commit 1175a4b into ansible-collections:main Jan 25, 2022
@mandar242 mandar242 deleted the 366_ec2_vol_outpostarn branch January 25, 2022 21:47
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
Add keys_attr parameter to aws_kms_info

SUMMARY
Move the kms_info return value from "keys" (which conflicts with keys()) to kms_keys.  Add a parameter to support the old version
Fixes ansible-collections#597
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_kms_info
ADDITIONAL INFORMATION

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for OutpostArn in ec2_vol
5 participants