-
-
Notifications
You must be signed in to change notification settings - Fork 796
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
Support httpApi payload override on handler #1312
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shalvah great thanks for that PR. It's a great call, still please see my comment
src/ServerlessOffline.js
Outdated
? service.provider.httpApi.payload | ||
: '2.0' | ||
if (functionDefinition.httpApi && functionDefinition.httpApi.payload) { | ||
httpEvent.http.payload = functionDefinition.httpApi.payload; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe in that case eventual setting on httpEvent
should take precedence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
httpEvent
? Do you mean the provider setting? If so, isn't that counterintuitive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, ignore my comment. There's no setting on event directly (it obviously wouldn't make sense :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you ! That's a great fix
src/ServerlessOffline.js
Outdated
? service.provider.httpApi.payload | ||
: '2.0' | ||
if (functionDefinition.httpApi && functionDefinition.httpApi.payload) { | ||
httpEvent.http.payload = functionDefinition.httpApi.payload; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, ignore my comment. There's no setting on event directly (it obviously wouldn't make sense :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shalvah I wanted to merge it, but CI reported an issue with style formatting.
All changes need to respect Prettier formatting. Can you update the PR?
Fixed. I imagine there may be some other overrides currently being overlooked like this. I'll send in a PR if I come across any. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Description
This PR adds support for getting the payload version for
httpApi
functions from the function definition (ie overriding theprovider
level).Motivation and Context
According to the serverless docs, you can specify
httpApi.payload
at either the service or function level. However, serverless-offline was ignoring the value set at the function level. This PR fixes that.How Has This Been Tested?
I've tested this in my current project.