feat(apigatewayv2-integrations): sqs integrations #29646

badmintoncryer · 2024-03-29T04:20:54Z

Issue # (if applicable)

Closes #24785.

Reason for this change

HTTP API supports AWS service integrations with SQS but CDK does not support this.

Description of changes

Added HttpSqsIntegration class

Description of how you validated changes

I've added both unit and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

TheRealAmazonKendra

Thank you for your contribution! I really like the thought put in here to default values and abstracting away some of the parameters you know users will need every time. I have a few questions/comments inline regarding the contract. I'm not sure it's quite right but this is a great start.

TheRealAmazonKendra · 2024-04-10T23:06:45Z

packages/aws-cdk-lib/aws-apigatewayv2-integrations/lib/http/sqs.ts

+   *
+   * @default HttpIntegrationSubtype.SQS_SEND_MESSAGE
+   */
+  readonly subtype?: apigwv2.HttpIntegrationSubtype;


Since only a couple of these are relevant here, I think that this isn't quite the right type to use here. Of the 10 values, only 4 work.

Additionally, per the comments above where you mention that certain arns need to be added based off the subtype here, I think that these props aren't quite the right contract in general. For instance, in the case that this subtype is START_EXECUTION, the parameter mapping shouldn't be optional because it must include at least the StateMachineArn.

Wait, I actually went back and read through the SubTypes more and it looks like the subtypes listed in the parameterMapping docstring above don't start with SQS_ so those comments don't mesh with the ones here in subType.

I'm really sorry for describing crazy comments for parameterMapping.
I have updated it and could you please re-confirm it?

Why not narrow down the type to the sane choices? Ah damn, it's an enum...

type PickEnum<T, K extends T> = { [P in keyof K]: P extends K ? P : never; }; type HttpSqsIntegrationProps = { readonly subtype: PickEnum<HttpIntegrationSubtype, | HttpIntegrationSubtype.SQS_SEND_MESSAGE | HttpIntegrationSubtype.SQS_RECEIVE_MESSAGE | HttpIntegrationSubtype.SQS_DELETE_MESSAGE | HttpIntegrationSubtype.SQS_PURGE_QUEUE >; readonly parameterMapping?: ParameterMapping; };

Would that work?

@chriswilty Thank you for your suggestion.
This PickEnum type will work correctly in TypeScript. However, it may not function properly when converted to other languages using jsii.
For example, I believe even TypeScript's union types couldn't be used in CDK.

I would like to check with the maintainer if it is usable.

IMO, I think we can keep this since there's validation below that checks the subtype values.

TheRealAmazonKendra · 2024-04-10T23:09:35Z

packages/aws-cdk-lib/aws-apigatewayv2-integrations/lib/http/sqs.ts

+   *
+   * @see https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-parameter-mapping.html
+   *
+   * @default - specify only `StateMachineArn`


Below in the code you're using this.props.queue.queueUrl so this doesn't seem accurate.

I have fixed it.

TheRealAmazonKendra · 2024-04-10T23:10:55Z

packages/aws-cdk-lib/aws-apigatewayv2-integrations/lib/http/sqs.ts

+   *
+   * @default - specify only `StateMachineArn`
+   */
+  readonly parameterMapping?: apigwv2.ParameterMapping;


Are these parameter mappings typically defined or can they be whatever the user adds here?

Default parameter mappings are based on this docs.
I defined only required parameter such as QueueUrl for SQS_SEND_MESSAGE.

If a user wants to set optional parameters or change the values of required parameters, it is assumed that they will provide a custom-defined parameterMapping.

Is this response addressing your question? Please feel free to correct me if my understanding is incorrect.

Pull request has been modified.

badmintoncryer · 2024-04-13T13:42:40Z

@TheRealAmazonKendra Thank you for your review! I deeply apologize for the nonsensical comments regarding the parameterMappings. (It seems that it was likely mistakenly copied from somewhere...)
I have corrected the relevant sections and responded to your questions. Please review at your convenience.

packages/aws-cdk-lib/aws-apigatewayv2-integrations/README.md

aws-cdk-automation · 2024-06-03T00:09:22Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

GavinZZ · 2025-02-06T21:55:52Z

packages/aws-cdk-lib/aws-apigatewayv2-integrations/lib/http/sqs.ts

+   *
+   * @default HttpIntegrationSubtype.SQS_SEND_MESSAGE
+   */
+  readonly subtype?: apigwv2.HttpIntegrationSubtype;


IMO, I think we can keep this since there's validation below that checks the subtype values.

mergify · 2025-02-06T21:57:12Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

GavinZZ · 2025-02-06T21:57:31Z

Going to approve on behalf of Kendra since this PR has been sitting here for almost half a year.

mergify · 2025-02-06T22:02:07Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2025-02-07T00:05:13Z

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

Pull request has been modified.

GavinZZ · 2025-02-07T21:12:14Z

@badmintoncryer can you fix the linting errors when available?

Pull request has been modified.

badmintoncryer · 2025-02-07T23:20:31Z

@GavinZZ Thank you for your approval. I've fixed linting errors.

mergify · 2025-02-10T18:42:34Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2025-02-10T18:47:33Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2025-02-10T19:25:48Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: dbd315a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2025-02-10T19:26:22Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2025-02-10T19:26:40Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels Mar 29, 2024

aws-cdk-automation requested a review from a team March 29, 2024 04:21

aws-cdk-automation previously requested changes Mar 29, 2024

View reviewed changes

badmintoncryer marked this pull request as ready for review April 1, 2024 03:07

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Apr 1, 2024

github-actions bot mentioned this pull request Apr 1, 2024

Weekly PR metrics report #29673

Closed

TheRealAmazonKendra previously requested changes Apr 10, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Apr 10, 2024

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Apr 13, 2024

chriswilty reviewed May 20, 2024

View reviewed changes

packages/aws-cdk-lib/aws-apigatewayv2-integrations/README.md Outdated Show resolved Hide resolved

badmintoncryer force-pushed the 24785-sqsSendMessageIntegration branch from d3c3c88 to d8a7760 Compare June 3, 2024 15:07

badmintoncryer added 12 commits June 4, 2024 00:08

feat: sqs integration

88b1d5b

fix: bug

2a7cb4a

test: add integ test

89d68dd

test: update integ test

6e67855

test: add unit test

0e9a3b1

fix: import order

66e405d

docs: update readme

470e0e2

docs: update readme

fc1fb78

docs: update readme

76ccddc

docs: update readme

26ecbd4

chore: udpate comments

02e2218

docs: update readme

73ad0d0

badmintoncryer added 2 commits December 31, 2024 23:31

Merge branch 'main' into 24785-sqsSendMessageIntegration

6ea65f5

Merge branch 'main' into 24785-sqsSendMessageIntegration

e1fd912

GavinZZ previously approved these changes Feb 6, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 6, 2025

Merge branch 'main' into 24785-sqsSendMessageIntegration

901802b

GavinZZ previously approved these changes Feb 7, 2025

View reviewed changes

use ValidationError

c5b4ad7

Merge branch 'main' into 24785-sqsSendMessageIntegration

11e4b06

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 7, 2025

badmintoncryer added 2 commits February 8, 2025 08:27

Merge branch 'main' into 24785-sqsSendMessageIntegration

4fd55ad

Merge branch 'main' into 24785-sqsSendMessageIntegration

730ce52

GavinZZ approved these changes Feb 10, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 10, 2025

Merge branch 'main' into 24785-sqsSendMessageIntegration

dbd315a

mergify bot merged commit 775c009 into aws:main Feb 10, 2025
20 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2025

badmintoncryer deleted the 24785-sqsSendMessageIntegration branch February 10, 2025 22:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(apigatewayv2-integrations): sqs integrations #29646

feat(apigatewayv2-integrations): sqs integrations #29646

badmintoncryer commented Mar 29, 2024 •

edited

Loading

aws-cdk-automation left a comment

TheRealAmazonKendra left a comment

TheRealAmazonKendra Apr 10, 2024

TheRealAmazonKendra Apr 10, 2024

badmintoncryer Apr 13, 2024

chriswilty May 20, 2024 •

edited

Loading

badmintoncryer May 21, 2024

GavinZZ Feb 6, 2025

TheRealAmazonKendra Apr 10, 2024

badmintoncryer Apr 13, 2024

TheRealAmazonKendra Apr 10, 2024

badmintoncryer Apr 13, 2024

badmintoncryer commented Apr 13, 2024

aws-cdk-automation commented Jun 3, 2024

GavinZZ Feb 6, 2025

mergify bot commented Feb 6, 2025

GavinZZ commented Feb 6, 2025

mergify bot commented Feb 6, 2025

mergify bot commented Feb 7, 2025

GavinZZ commented Feb 7, 2025

badmintoncryer commented Feb 7, 2025

mergify bot commented Feb 10, 2025

mergify bot commented Feb 10, 2025

aws-cdk-automation commented Feb 10, 2025

mergify bot commented Feb 10, 2025

github-actions bot commented Feb 10, 2025

feat(apigatewayv2-integrations): sqs integrations #29646

feat(apigatewayv2-integrations): sqs integrations #29646

Conversation

badmintoncryer commented Mar 29, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriswilty May 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

badmintoncryer commented Apr 13, 2024

aws-cdk-automation commented Jun 3, 2024

Choose a reason for hiding this comment

mergify bot commented Feb 6, 2025

GavinZZ commented Feb 6, 2025

mergify bot commented Feb 6, 2025

mergify bot commented Feb 7, 2025

GavinZZ commented Feb 7, 2025

badmintoncryer commented Feb 7, 2025

mergify bot commented Feb 10, 2025

mergify bot commented Feb 10, 2025

aws-cdk-automation commented Feb 10, 2025

AWS CodeBuild CI Report

mergify bot commented Feb 10, 2025

github-actions bot commented Feb 10, 2025

badmintoncryer commented Mar 29, 2024 •

edited

Loading

chriswilty May 20, 2024 •

edited

Loading