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

feat(apigatewayv2): introduce new interface for integration with sfn tasks and r53 targets #14202

Closed
wants to merge 2 commits into from

Conversation

NetaNir
Copy link
Contributor

@NetaNir NetaNir commented Apr 16, 2021

stepfunctions-tasks and route53-targets are stable modules, meaning that in v2 they can not have a dependency on experimental modules (to avoid cyclic dependencies). Currently both depends on apigatewayv2. This PR breaks the two dependencies by introducing the appropriate interface in each module.

Note that since these are breaking changes to stable modules, they can not be merged to master.


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

@gitpod-io
Copy link

gitpod-io bot commented Apr 16, 2021

@NetaNir NetaNir requested a review from a team April 16, 2021 04:37
@github-actions github-actions bot added the @aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 label Apr 16, 2021
@NetaNir NetaNir requested review from shivlaks and nija-at April 16, 2021 04:37
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Apr 16, 2021
Comment on lines -16 to +25
readonly api: apigatewayv2.IHttpApi;
readonly api: IApiGatewayV2HttpApi;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just make this -

readonly apiId: string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Because of the way it is used in the consumer. The consumer code requires the apigw containing Construct in order to get the defining Stack to get its region:
const apiStack = cdk.Stack.of(this.props.api);
return `${this.props.api.apiId}.execute-api.${apiStack.region}.${apiStack.urlSuffix}`;

The way to express it with an interface is to have it extends IConstruct which will enforce the requirements of a scope.

Since we decided to merge this change to master (in the chat), keeping the interface has the advantage of not breaking typescript users.

…-name.ts

Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: c8e84ba
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@NetaNir
Copy link
Contributor Author

NetaNir commented Apr 17, 2021

closed in favor of #14227

@NetaNir NetaNir closed this Apr 17, 2021
@rix0rrr rix0rrr deleted the neta/apigwv2-dep branch January 5, 2022 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants