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): http api - custom domain & stage mapping #8027

Merged
merged 58 commits into from
Jul 6, 2020
Merged

feat(apigatewayv2): http api - custom domain & stage mapping #8027

merged 58 commits into from
Jul 6, 2020

Conversation

pahud
Copy link
Contributor

@pahud pahud commented May 16, 2020

  • implementation
  • README
  • integ test
  • 100% unit test coverage

Commit Message

feat(apigatewayv2): http api - custom domain & stage mapping (#8027)

  • Add new DomainName and HttpApiMapping construct classes and addDomainName() method for HttpApi resource.
  • Add defaultDomainMapping construct property for HttpApi
  • Add domainMapping attribute for addStage

Closes #7847

End Commit Message


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

@pahud pahud marked this pull request as draft May 16, 2020 03:18
@pahud pahud marked this pull request as ready for review May 16, 2020 05:54
@pahud pahud marked this pull request as draft May 16, 2020 15:53
@pahud pahud marked this pull request as ready for review May 17, 2020 07:22
@pahud
Copy link
Contributor Author

pahud commented May 18, 2020

Hi @nija-at

I am ready now :-)

Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

It seems that HttpApiMapping and DomainName constructs are common to both Websocket and HTTP APIs. I can't find any properties that are available to one but not the other.

Let's move these to the common/ folder instead so they can be shared.

You may have to create an IApi interface that will sit between IHttpApi and IResource as -

export interface IApi extends IResource {
  ...
}

export IHttpApi extends IApi {
  ...
}

@nija-at nija-at changed the title feat(apigatewayv2): support custom domain feat(apigatewayv2): http api - custom domain & stage mapping May 19, 2020
@pahud pahud marked this pull request as draft May 21, 2020 15:12
@mergify mergify bot dismissed nija-at’s stale review May 21, 2020 15:26

Pull request has been modified.

@pahud pahud marked this pull request as ready for review May 21, 2020 17:57
@pahud
Copy link
Contributor Author

pahud commented May 21, 2020

Hi @nija-at

As there's only one attribute in IHttpApi, I am not sure if we should make a IApi between IHttpApi and IResource in this PR. But I've moved some interfaces of domain name and api mapping into common. Please kindly review it again.

export interface IHttpApi extends IResource {
/**
* The identifier of this API Gateway HTTP API.
* @attribute
*/
readonly httpApiId: string;
}

@pahud pahud requested a review from nija-at May 21, 2020 18:17
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Thanks @pahud. Some comments below -

packages/@aws-cdk/aws-apigatewayv2/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigatewayv2/lib/http/api-mapping.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigatewayv2/README.md Outdated Show resolved Hide resolved
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@mergify mergify bot dismissed nija-at’s stale review June 29, 2020 15:10

Pull request has been modified.

pahud and others added 6 commits June 29, 2020 23:11
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@pahud pahud marked this pull request as draft June 29, 2020 15:36
@pahud pahud marked this pull request as ready for review June 29, 2020 15:57
nija-at
nija-at previously requested changes Jul 2, 2020
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Hey Pahud. A few things to take care of and from the previous comments.

Should be easy and this should be ready to go 😊

pahud and others added 2 commits July 3, 2020 09:38
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@mergify mergify bot dismissed nija-at’s stale review July 3, 2020 01:38

Pull request has been modified.

@pahud
Copy link
Contributor Author

pahud commented Jul 3, 2020

@nija-at all comments should be all sorted but my laptop can't build it successfully. I'm still trying to get it sorted. FYI.

@pahud
Copy link
Contributor Author

pahud commented Jul 3, 2020

OK I am ready.

@pahud pahud requested a review from nija-at July 6, 2020 04:58
@mergify
Copy link
Contributor

mergify bot commented Jul 6, 2020

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 4fa394c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Jul 6, 2020

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

@mergify mergify bot merged commit 5e43348 into aws:master Jul 6, 2020
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.

apigatewayv2 HTTP API to support custom domain name
3 participants