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

WIP: Get request templates working #4

Closed
wants to merge 3 commits into from
Closed

WIP: Get request templates working #4

wants to merge 3 commits into from

Conversation

johncmckim
Copy link
Collaborator

What did you implement:

Closes #3

How did you implement it:

I have added helpers for velocity templates in ./lib/integration/velocity. These helpers create the context and render the velocity templates.

The lambda integration executes the velocity template on the request and maps the response based on the error.

This is currently using the default templates. Accessing the config should be easier once serverless/serverless#2510 is merged

How can we verify it:

  • Execute the tests
  • Make requests against the examples project - cd examples && npm start

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Provide verification config/commands/resources
  • Change ready for review message below

Is this ready for review?: NO

@gertjvr
Copy link
Collaborator

gertjvr commented Nov 1, 2016

Awesome

@johncmckim
Copy link
Collaborator Author

@gertjvr I've added really basic support for the lambda integration and VTL templates. It just uses the default templates it doesn't currently support templates set by users. What do you think of adding partial support now until #2554 can be looked at?

@gertjvr
Copy link
Collaborator

gertjvr commented Nov 8, 2016

Will have a look tonight, see if we can merge these changes.

Thank you for working on this.

@johncmckim
Copy link
Collaborator Author

No worries. I need to add some more unit tests to cover the changes I made today. But it would be great if you could take a look at the changes as they are now.

P.S. I noticed there's some whitespace changes. Not sure how that's happened but if you add ?w=0 to the query string of the compare view it will remove whitespace changes from review.

if (context.integration === 'lambda') {
context.requestTemplate = velocityDefaults.JSON_REQUEST_TEMPLATE
context.responseMappings = velocityDefaults.RESPONSE_STATUS_CODES
}
Copy link
Collaborator

@gertjvr gertjvr Nov 15, 2016

Choose a reason for hiding this comment

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

Wondering if this can be pushed into the lambda integration as we are just picking these values of the context in the request and response integration methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I placed it there as once it is possible to get the configuration from the function object this is where it would be configured.

But I am happy to move it into lambda.js if you would prefer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense now.

@johncmckim
Copy link
Collaborator Author

Closing in favour of #19

@johncmckim johncmckim closed this Feb 11, 2017
@johncmckim johncmckim deleted the issue-3 branch February 13, 2017 22:25
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.

None yet

2 participants