lambda_event: Add support for FunctionResponseTypes #1209
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
|
cc @jillr @pjodouin @ryansb @s-hertel @tremble |
ansibullbot
added
community_review
feature
This comment was marked as outdated.
This comment was marked as outdated.
abikouo
reviewed
Oct 27, 2022
| - (Streams and Amazon SQS) A list of current response type enums applied to the event source mapping. | ||
| type: list | ||
| elements: str | ||
| choices: [ReportBatchItemFailures] |
There was a problem hiding this comment.
only one choice and using a list of strings?
Could you please update the parameter and define it as a boolean?
There was a problem hiding this comment.
This is coming from the AWS spec. Presumably, they are doing this with the intent that there will be more options in the future. I think we should leave this as is, because it will be much easier to add a new item to this list than to change the parameter type altogether in the future.
There was a problem hiding this comment.
@abikouo , yes assuming AWS will come up with more choices in future, I kept it as per boto3 documentation.
This comment was marked as outdated.
This comment was marked as outdated.
mandar242
force-pushed
the
lambda_event-add_FunctionResponseTypes_support
branch
from
3b42a31
to
e81cb19
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
@mandar242 I think we should add a testcase to test the newly added key. |
| @@ -0,0 +1,2 @@ | |||
| minor_changes: | |||
| - lambda_event - Added support to set FunctionResponseTypes when creating lambda event source mappings (https://github.com/ansible-collections/amazon.aws/pull/1209). | |||
There was a problem hiding this comment.
Suggested change
| - lambda_event - Added support to set FunctionResponseTypes when creating lambda event source mappings (https://github.com/ansible-collections/amazon.aws/pull/1209). | |
| - lambda_event - Added support to set ``function_response_types`` when creating lambda event source mappings (https://github.com/ansible-collections/amazon.aws/pull/1209). |
There was a problem hiding this comment.
@GomathiselviS @alinabuzachis currently I don't think we have integration tests for lambda_event, would it be fine to add tests for this PR under integration tests for lambda https://github.com/ansible-collections/amazon.aws/tree/main/tests/integration/targets/lambda ?
There was a problem hiding this comment.
I would probably go with a new test suite for lambda_event, but it's up to you.
There was a problem hiding this comment.
I feel if it's okay, I'd like to get this PR merged first and open new PR for adding test suite?
@alinabuzachis @gravesm @GomathiselviS
There was a problem hiding this comment.
In general, we should really be adding tests along with the code. Tests can often highlight problems with a PR that aren't obvious by just reviewing. There's also the danger that even with the best intentions, the tests never get added later due to any number of reasons.
There was a problem hiding this comment.
Okay, will add test suite to this PR soon.
The reason I was thinking to create another PR for test suite is that, I might be wrong but it does require significant amount of setup for testing lambda_event, need a stream (eg dynamodb table, stream), lambda function, IAM role for proper policies for lambda function.
mandar242
force-pushed
the
lambda_event-add_FunctionResponseTypes_support
branch
from
e81cb19
to
492a19f
Compare
|
recheck |
3 similar comments
|
recheck |
|
recheck |
|
recheck |
|
recheck |
|
Sorry, I forgot to remove the Depends-On: ansible/ansible-zuul-jobs#1691 |
|
recheck |
|
recheck |
gravesm
reviewed
Nov 14, 2022
gravesm
approved these changes
Nov 16, 2022
goneri
approved these changes
Nov 16, 2022
GomathiselviS
approved these changes
Nov 16, 2022
alinabuzachis
pushed a commit
to alinabuzachis/amazon.aws
that referenced
this pull request
Apr 27, 2023
…ons#1209) lambda_event: Add support for FunctionResponseTypes SUMMARY Resolves ansible-collections#1081 Added support to set FunctionResponseTypes when creating lambda event source mappings ISSUE TYPE Feature Pull Request COMPONENT NAME lambda_event ADDITIONAL INFORMATION Can be tested using below playbook (needs a dynamo db table and lambda function with appropriate access/policy/role) --- - name: lambda_event hosts: localhost gather_facts: false tasks: - name: Create DynamoDB stream event mapping (trigger) amazon.aws.lambda_event: state: present event_source: stream function_name: my-test-function source_params: source_arn: arn:aws:dynamodb:us-east-2:721234567890:table/my-test-table/stream/2022-10-26T17:55:25.232 enabled: True batch_size: 500 starting_position: LATEST function_response_types: - ReportBatchItemFailures register: event - name: Get info on above trigger command: 'aws lambda get-event-source-mapping --uuid {{ event.events.uuid }}' environment: AWS_ACCESS_KEY_ID: "{{ aws_access_key }}" AWS_SECRET_ACCESS_KEY: "{{ aws_secret_key }}" AWS_SESSION_TOKEN: "{{ security_token | default('') }}" AWS_DEFAULT_REGION: "{{ aws_region }}" register: my_function_details - name: convert it to an object set_fact: my_function_details_obj: "{{ my_function_details.stdout | from_json }}" - assert: that: - my_function_details_obj.FunctionResponseTypes is defined - my_function_details_obj.FunctionResponseTypes | length > 0 - my_function_details_obj.FunctionResponseTypes[0] == "ReportBatchItemFailures" Reviewed-by: Bikouo Aubin <None> Reviewed-by: Mike Graves <mgraves@redhat.com> Reviewed-by: Mandar Kulkarni <mandar242@gmail.com> Reviewed-by: Alina Buzachis <None> Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net> Reviewed-by: GomathiselviS <None>
softwarefactory-project-zuul bot
pushed a commit
that referenced
this pull request
Apr 28, 2023
[manual backport stable-5] lambda_event: Add support for FunctionResponseTypes (#1209) lambda_event: Add support for FunctionResponseTypes SUMMARY Resolves #1081 Added support to set FunctionResponseTypes when creating lambda event source mappings ISSUE TYPE Feature Pull Request COMPONENT NAME lambda_event ADDITIONAL INFORMATION Can be tested using below playbook (needs a dynamo db table and lambda function with appropriate access/policy/role) name: lambda_event hosts: localhost gather_facts: false tasks: - name: Create DynamoDB stream event mapping (trigger) amazon.aws.lambda_event: state: present event_source: stream function_name: my-test-function source_params: source_arn: arn:aws:dynamodb:us-east-2:721234567890:table/my-test-table/stream/2022-10-26T17:55:25.232 enabled: True batch_size: 500 starting_position: LATEST function_response_types: - ReportBatchItemFailures register: event name: Get info on above trigger command: 'aws lambda get-event-source-mapping --uuid {{ event.events.uuid }}' environment: AWS_ACCESS_KEY_ID: "{{ aws_access_key }}" AWS_SECRET_ACCESS_KEY: "{{ aws_secret_key }}" AWS_SESSION_TOKEN: "{{ security_token | default('') }}" AWS_DEFAULT_REGION: "{{ aws_region }}" register: my_function_details name: convert it to an object set_fact: my_function_details_obj: "{{ my_function_details.stdout | from_json }}" assert: that: - my_function_details_obj.FunctionResponseTypes is defined - my_function_details_obj.FunctionResponseTypes | length > 0 - my_function_details_obj.FunctionResponseTypes[0] == "ReportBatchItemFailures" Reviewed-by: Bikouo Aubin Reviewed-by: Mike Graves mgraves@redhat.com Reviewed-by: Mandar Kulkarni mandar242@gmail.com Reviewed-by: Alina Buzachis Reviewed-by: Gonéri Le Bouder goneri@lebouder.net Reviewed-by: GomathiselviS SUMMARY ISSUE TYPE Bugfix Pull Request Docs Pull Request Feature Pull Request New Module Pull Request COMPONENT NAME ADDITIONAL INFORMATION
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
…`` (ansible-collections#1212) ecs_* - fix idempotence bug in ecs_service and dont require ``cluster`` SUMMARY Don't require cluster param and use cluster name 'default' when not specified (see docs). Fix bug when comparing health_check_grace_period_seconds when not input by user. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ecs_service ecs_task ADDITIONAL INFORMATION Split up from ansible-collections#1209 to backport to stable-2 Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
SUMMARY
Add wait parameter to utilize boto3 waiters in ecs_service and ecs_task (ServicesInactive, TasksStopped, TasksRunning).
There's an additional waiter for ServicesStable but idempotence checked never failed locally so it seems redundant when creating a service.
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
ecs_service
ecs_task
ADDITIONAL INFORMATION
When testing the waiter for TasksRunning, tests failed on waiter error due to the container instance not being able to be created, not because of the waiter, so I commented out those tests for now.
In the ECS console:
Stopped reason CannotPullContainerError: inspect image has been retried 5 time(s): failed to resolve ref "docker.io/library/nginx:latest": failed to do request: Head https://registry-1.docker.io/v2/library/nginx/manifests/latest: dial tcp 34.237.244.67:443: i/o timeout
* add waiters and fix some bugs
* add changelog
* move bugfixes to different PR for backporting purposes
* update wait description
* catch WaiterError
* Bump version_added
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
…`` (ansible-collections#1212) ecs_* - fix idempotence bug in ecs_service and dont require ``cluster`` SUMMARY Don't require cluster param and use cluster name 'default' when not specified (see docs). Fix bug when comparing health_check_grace_period_seconds when not input by user. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ecs_service ecs_task ADDITIONAL INFORMATION Split up from ansible-collections#1209 to backport to stable-2 Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Sep 18, 2023
SUMMARY
Add wait parameter to utilize boto3 waiters in ecs_service and ecs_task (ServicesInactive, TasksStopped, TasksRunning).
There's an additional waiter for ServicesStable but idempotence checked never failed locally so it seems redundant when creating a service.
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
ecs_service
ecs_task
ADDITIONAL INFORMATION
When testing the waiter for TasksRunning, tests failed on waiter error due to the container instance not being able to be created, not because of the waiter, so I commented out those tests for now.
In the ECS console:
Stopped reason CannotPullContainerError: inspect image has been retried 5 time(s): failed to resolve ref "docker.io/library/nginx:latest": failed to do request: Head https://registry-1.docker.io/v2/library/nginx/manifests/latest: dial tcp 34.237.244.67:443: i/o timeout
* add waiters and fix some bugs
* add changelog
* move bugfixes to different PR for backporting purposes
* update wait description
* catch WaiterError
* Bump version_added
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Oct 24, 2023
…`` (ansible-collections#1212) ecs_* - fix idempotence bug in ecs_service and dont require ``cluster`` SUMMARY Don't require cluster param and use cluster name 'default' when not specified (see docs). Fix bug when comparing health_check_grace_period_seconds when not input by user. ISSUE TYPE Bugfix Pull Request COMPONENT NAME ecs_service ecs_task ADDITIONAL INFORMATION Split up from ansible-collections#1209 to backport to stable-2 Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Alina Buzachis <None>
abikouo
pushed a commit
to abikouo/amazon.aws
that referenced
this pull request
Oct 24, 2023
SUMMARY
Add wait parameter to utilize boto3 waiters in ecs_service and ecs_task (ServicesInactive, TasksStopped, TasksRunning).
There's an additional waiter for ServicesStable but idempotence checked never failed locally so it seems redundant when creating a service.
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
ecs_service
ecs_task
ADDITIONAL INFORMATION
When testing the waiter for TasksRunning, tests failed on waiter error due to the container instance not being able to be created, not because of the waiter, so I commented out those tests for now.
In the ECS console:
Stopped reason CannotPullContainerError: inspect image has been retried 5 time(s): failed to resolve ref "docker.io/library/nginx:latest": failed to do request: Head https://registry-1.docker.io/v2/library/nginx/manifests/latest: dial tcp 34.237.244.67:443: i/o timeout
* add waiters and fix some bugs
* add changelog
* move bugfixes to different PR for backporting purposes
* update wait description
* catch WaiterError
* Bump version_added
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Resolves #1081
Added support to set
FunctionResponseTypeswhen creating lambda event source mappingsISSUE TYPE
COMPONENT NAME
lambda_event
ADDITIONAL INFORMATION
Can be tested using below playbook (needs a dynamo db table and lambda function with appropriate access/policy/role)