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.

Sign up for GitHub

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

Jump to bottom

feat(aws-apigatewayv2websocket-sqs #1140

Merged
merged 31 commits into from
Jul 19, 2024
Merged

feat(aws-apigatewayv2websocket-sqs #1140

merged 31 commits into from
Jul 19, 2024

Conversation

biffgaut
Copy link
Contributor

Issue #, if available:

Description of changes:
New construct

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: b177e44
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 47b9a88
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 42eb94b
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 5ef953b
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 380bb00
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: b98553d
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 2769324
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 6465b21
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 4237dd8
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

biffgaut
biffgaut commented Jul 11, 2024
View reviewed changes
source/patterns/@aws-solutions-constructs/aws-apigatewayv2websocket-sqs/README.md Outdated
|allowCreateOperation?|`boolean`|Whether to deploy an API Gateway Method for POST HTTP operations on the queue (i.e. sqs:SendMessage).|
|createRequestTemplate?|`string`|API Gateway Request Template for the create method for the default `application/json` content-type. This property is required if the `allowCreateOperation` property is set to true.|
|additionalCreateRequestTemplates?|`{ [contentType: string]: string; }`|Optional Create Request Templates for content-types other than `application/json`. Use the `createRequestTemplate` property to set the request template for the `application/json` content-type. This property can only be specified if the `allowCreateOperation` property is set to true.|
|createIntegrationResponses?|[`api.IntegrationResponses[]`](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_apigateway.IntegrationResponse.html)|Optional, custom API Gateway Integration Response for the create method. This property can only be specified if the `allowCreateOperation` property is set to true.|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll move forward as is, but the PR I just pushed added the ability to provide MethodResponses along with IntegrationResponses. I will update this as a follow up in a different PR. Important question - are there things I should know about MethodResponses with websockets API as opposed to a RestApi?

source/patterns/@aws-solutions-constructs/aws-apigatewayv2websocket-sqs/README.md Outdated Show resolved Hide resolved
source/patterns/@aws-solutions-constructs/aws-apigatewayv2websocket-sqs/README.md Outdated Show resolved Hide resolved
source/patterns/@aws-solutions-constructs/aws-apigatewayv2websocket-sqs/lib/index.ts Outdated

const buildWetbSocketQueueApiResponse = defaults.buildWebSocketQueueApi(this, id, {
queue: this.sqsQueue,
// TODO: Should we create an interface representing { [contentType: string]: string; }? It would be in code documentation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't see the relevance of this TODO anymore - did you address it by refactoring in other ways?

Or did the code it refers to get moved to core? I'll look for it there.

source/patterns/@aws-solutions-constructs/core/lib/websocket-api-helper.ts Outdated Show resolved Hide resolved
source/patterns/@aws-solutions-constructs/core/lib/websocket-api-helper.ts Show resolved Hide resolved
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 345ab1b
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@knihit
update README to remove authorizer reference. An authorizer can only …
b7b976e 
…be set with and which is not created by this construct
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: df70ddb
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 3764956
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 9b69b74
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: b457483
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 9243763
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 09c8b46
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 0c1f138
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: c37f5c1
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: f5d5f85
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

biffgaut
biffgaut commented Jul 18, 2024
View reviewed changes
Copy link
Contributor Author

@biffgaut biffgaut left a comment

Choose a reason for hiding this comment

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

A couple questions (that may lead to changes)

source/patterns/@aws-solutions-constructs/aws-apigatewayv2websocket-sqs/lib/index.ts Outdated Show resolved Hide resolved
source/patterns/@aws-solutions-constructs/aws-apigatewayv2websocket-sqs/lib/index.ts Outdated
@@ -96,7 +96,6 @@ export interface ApiGatewayV2WebSocketToSqsProps {
/**
* API Gateway Request Template for the default route.
*
* @default - "Action=ReceiveMessage"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there no default now? If it is required if createDefaultRoute is true then we should state that in the comments (and check it at the top of the constructor)

Copy link
Member

Choose a reason for hiding this comment

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

No, its not required to create the default route. In fact, you may not want to have a default. So if a message action mapping is not found on the socket, it better to throw an error rather than map it to a default. Depending on the use case, creating $default should be optional.

source/patterns/@aws-solutions-constructs/core/lib/websocket-api-helper.ts Outdated
};

printWarning("defaultIamAuthorization is set to false. This construct will not add any authorizers. If this is not the case, please pass the `connectRouteOptions` with an authorizer or call `addRoute` on the websocket endpoint and provide the `$connect` configuration with an authorizer");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see the condition that checks whether to print this or not, looks like it is printed always?

And how is this passing eslint with a line almost 300 characters long?

@knihit
incorporate review comments, fix typo in the README, add condition fo…
b9512e4 
…r printWarning call and update the log message inthe printWarning call
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: b9512e4
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: e0196b4
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@knihit
add custom route feature support to the construct, added unit tests, …
1e75acf 
…integ tests and updated README.md with the new prop
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 1e75acf
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

georgebearden
georgebearden approved these changes Jul 19, 2024
View reviewed changes
Copy link
Contributor

@georgebearden georgebearden left a comment

Choose a reason for hiding this comment

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

looks good to me

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: eba7d74
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@biffgaut biffgaut merged commit 5f9a9ed into main Jul 19, 2024
2 checks passed
@biffgaut biffgaut deleted the aws-apigwv2websocket-sqs branch July 19, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knihit knihit knihit left review comments

@georgebearden georgebearden georgebearden approved these changes

@hayesry hayesry Awaiting requested review from hayesry hayesry is a code owner

@mobri2a mobri2a Awaiting requested review from mobri2a mobri2a is a code owner

@emcfins emcfins Awaiting requested review from emcfins emcfins is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@biffgaut @aws-solutions-constructs-team @georgebearden @knihit