-
Notifications
You must be signed in to change notification settings - Fork 34
Add Codable helpers for FunctionURL, APIGatewayV2, and SQS #90
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
Conversation
Thank you @natanrolnik for this welcome change. |
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 so much for this addition. I left one question and one suggestion. Also, be sure to include the license header on all source files and apply the swift formatting (there is a script to help you in ./scripts
format.sh`)
Sources/AWSLambdaEvents/Codable Helpers/APIGatewayV2+Encode.swift
Outdated
Show resolved
Hide resolved
Sources/AWSLambdaEvents/Codable Helpers/APIGatewayV2+Decode.swift
Outdated
Show resolved
Hide resolved
Sources/AWSLambdaEvents/Codable Helpers/APIGatewayV2+Decode.swift
Outdated
Show resolved
Hide resolved
@sebsto done! I've unified them into two protocols: |
5fafa20
to
03ac6c9
Compare
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 or the changes. I still have two quetsions.
} | ||
} | ||
|
||
extension APIGatewayV2Response: EncodableResponse { |
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.
looks like the only difference between APIGAteway's init()
and the protocol is the order of the parameters. Unless I missed something else, is there a reason we can't use the existing order for the parameters and delete this piece of code.
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.
You're correct. I thought about changing the order, but it would be a breaking change.
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 agree that we can't change the order on the APIGatewayResponse, but can you change the order on the EncodableResponse
protocol ? That way APIGatewayResponse will automatically implement the EncodableResponse
protocol.
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.
Added this init()
function to allow this change
03ac6c9
to
cbc5418
Compare
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.
LGTM
Add helpers for FunctionURL, APIGatewayV2, and SQS events to ease usage of
Decodable
andEncodable
typesMotivation:
When using Function URL (or API Gateway) events and outputs, and SQS events, it is very common to decode a
Decodable
body, or encode aCodable
type into a function URL response. This PR adds convenient methods that are easy to use, but also maintain a level of customization by allowing custom JSON encoders/decoders.Modifications:
Encodable
types into aFunctionURLReponse
orAPIGatewayV2Response
Decodable
types from aFunctionURLRequest
orAPIGatewayV2Request
Decodable
s fromSQSEvent
records, or from a singleSQSEvent.Message
Result:
The changes allow the following usage:
Instead of:
The same is similar when decoding SQS event messages.
And when encoding a response:
Instead of: