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

SDK based API Gateway Stage updates #6084

Merged
merged 5 commits into from May 9, 2019
Merged

SDK based API Gateway Stage updates #6084

merged 5 commits into from May 9, 2019

Conversation

@pmuens
Copy link
Member

pmuens commented May 6, 2019

What did you implement:

Adds SDK based API Gateway Stage updates rather than going through CloudFormation compilations.

This way existing API Gateway stages can benefit from logging, tracing and tagging without the need to remove and re-deploy them.

"it just works..."

How did you implement it:

Call the SDK with the respective method to update the stage in an after:deploy:deploy hook.

How can we verify it:

service: test

provider:
  name: aws
  runtime: nodejs8.10
  # tags:
  #  foo: bar
  #  baz: qux
  # tracing:
  #   apiGateway: true
  # logs:
  #   restApi: true

functions:
  test:
    handler: handler.hello
    events:
      - http:
          method: GET
          path: hello

Deploy the service, uncomment the comments, re-deploy. Check the stage setup in the API Gateway console.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@pmuens pmuens added this to the 1.42.0 milestone May 6, 2019
@pmuens pmuens self-assigned this May 6, 2019
@pmuens pmuens added this to In progress in Serverless via automation May 6, 2019
@pmuens pmuens force-pushed the sdk-based-stage-updates branch 2 times, most recently from 83828d1 to c75d24d May 7, 2019
@pmuens

This comment has been minimized.

Copy link
Member Author

pmuens commented May 7, 2019

Just finished this PR and everything is working quite fine so far. We just need to test this more thoroughly with different setups.

Another bummer is that we cannot fully disable access logging. The SDK apparently doesn't support removal of the destinationArn and the log format. Additionally taggings won't be removed for now since this would require us to fetch the tags and match them against the ones the user wants to use. We should sync on whether tag removal is necessary and should be implemented as well.

@pmuens pmuens force-pushed the sdk-based-stage-updates branch 2 times, most recently from 9f821b9 to 69fa72a May 8, 2019
@pmuens pmuens added pr/in-review and removed pr/in-progress labels May 8, 2019
@pmuens pmuens marked this pull request as ready for review May 8, 2019
@pmuens

This comment has been minimized.

Copy link
Member Author

pmuens commented May 8, 2019

Just finalized this PR. Added the loggic so that unused variables / tags are removed.

The other issues mentioned above are apprently UI issues, so there's little we can do about that right now.

This PR is ready for a review and merge.

@pmuens pmuens requested review from dschep, medikoo and eahefnawy May 8, 2019
@pmuens pmuens force-pushed the sdk-based-stage-updates branch 4 times, most recently from 859bd18 to 49b74fe May 8, 2019
@pmuens pmuens force-pushed the sdk-based-stage-updates branch from 49b74fe to 1df8598 May 8, 2019
Copy link
Member

medikoo left a comment

@pmuens I put some suggestions, but it's nothing critical. If there's a need to get this in quickly, I think it's fine to skip on those, and reconsider those at some future point

@pmuens pmuens requested a review from medikoo May 9, 2019
Serverless automation moved this from In progress to Reviewer approved May 9, 2019
@medikoo
medikoo approved these changes May 9, 2019
@pmuens pmuens merged commit d9f6647 into master May 9, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Serverless automation moved this from Reviewer approved to Done May 9, 2019
@pmuens pmuens deleted the sdk-based-stage-updates branch May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Serverless
  
Done
2 participants
You can’t perform that action at this time.