Skip to content
This repository has been archived by the owner on Jul 23, 2021. It is now read-only.

RestApiId not set #9

Closed
thomasmichaelwallace opened this issue Dec 22, 2016 · 5 comments
Closed

RestApiId not set #9

thomasmichaelwallace opened this issue Dec 22, 2016 · 5 comments
Assignees
Labels

Comments

@thomasmichaelwallace
Copy link

I'm not sure if it's my configuration, but this plugin can't detect my RestApiId and therefore fails to update/create the documentation.

I've found a fix, which involves adding an alternative to assuming the the Cloud Formation stack has ApiId, as it can also be taken from the ServiceEndpoint.

I'll make a PR for discussion.

@tchock
Copy link
Contributor

tchock commented Dec 24, 2016

@thomasmichaelwallace Can you give more information about your configuration? Because normally the cloud formation stack should be established before the after deploy hook. But maybe it would be good to wait for completion first. Because your fix rather looks like a workaround.

I can't imagine the API ID not being there in general. I will have a look into this the next days (after Christmas).

@tchock tchock added the bug label Dec 24, 2016
@tchock tchock self-assigned this Dec 24, 2016
@thomasmichaelwallace
Copy link
Author

Sure,

It's quite possible I'm doing something wrong, however this turns out what I want- a lambda backed API Gateway call with a human readable url (serverless@1.4.0:):

service: api-hello

plugins:
  - serverless-webpack
  - serverless-aws-documentation

provider:
  .provider: &provider
    name: aws
    runtime: nodejs4.3
    stage: local

custom:
  BasePath: hello
  documentation:
    version: '1.0.0'
    models:
      -
        name: "HALResponse"
        description: "A response from HAL."
        contentType: "application/json"
        schema:
          title: HAL Response
          type: object
          properties:
            hal:
              type: string

functions:
  get:
    handler: hello.get
    description: GET /hello
    events:
      - http:
          method: get
          path: /
          cors: true
          documentation:
            summary: "Say hello to HAL"
            description: "Gets a response from HAL."
            methodResponses:
              -
                statusCode: 200
                responseModels:
                  "application/json": "HALResponse"

resources:
  Resources:
    APIDomainMapping:
      Type: AWS::ApiGateway::BasePathMapping
      Properties:
        BasePath: ${self:custom.BasePath}
        DomainName: ${env:API_URL_${opt:stage, self:provider.stage}}
        Stage: ${opt:stage, self:provider.stage}
        RestApiId:
          Ref: ApiGatewayRestApi

I take what you mean about it being more of a workaround.

From what I saw the cloud formation stack has been established by the time this code runs. It's just that the stacks don't include any with the key 'ApiId'. They include:

  • GetLambdaFunctionQualifiedArn
  • GetLambdaFunctionArn
  • ServiceEndpoint
  • ServerlessDeploymentBucketName

Maybe it's more reliable to query the resources, which include ApiGatewayRestApi?

It might also be worth noting that I was adding documentation to an API that had previously been created (e.g. it was a stack update, not a stack create); but surely this needs to be able to update docs on existing stacks too?

@tchock
Copy link
Contributor

tchock commented Dec 28, 2016

@thomasmichaelwallace I guess I found the problem that caused that bug.
In the project I was testing this, I added the api key to the outputs of cloud formation. The documentation part first looks for this api key, but it is not there when you don't add this to the output.

I tested this with the serverless.yml that you were providing and it worked when I added the key.
I will fix this today and give you notice in the PR, so you can test it out for yourself :)

@tchock
Copy link
Contributor

tchock commented Dec 28, 2016

Fixed the bug in #11. @thomasmichaelwallace: Can you have a look it this solves your problem?

EDIT:
I will merge this to master now. This fix was totally needed anyway. You just need to update the package from the npm repo.

@thomasmichaelwallace
Copy link
Author

Cheers @tchock that fixed it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants