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

Support creating a lambda directly from a ring handler #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gja
Copy link

@gja gja commented Apr 18, 2017

The main reason for this is that wrap-apigw-lambda-proxy does not confirm to the ring spec, especially since status is renamed to status code. It cannot be inserted at any position into the middleware stack. Hence, I wanted to create a single macro which will take a handler and create a lamda function. Since it's a macro, if any dependencies are not included, it will bomb at compile time

@mhjort
Copy link
Owner

mhjort commented Apr 20, 2017

You have very good point here. This is not a proper middleware because it works only when inserted into certain position in middleware stack. Thank you for the PR.

However, when using AWS Lambda you have to minimize dependencies whenever possible to keep the jar file size small. Therefore my original idea with this library was to have zero dependencies. Then the user of this library can choose what json parser he wants to use. For example I now sometimes use clojure.data.json parser instead of cheshire.

That said, would it make sense that macro would take json related functions as parameters instead of depending on cheshire directly? The dependency to lambada is something that maybe cannot be avoided. That could be then documented in README. What do you think?

@ricardoekm
Copy link

+1 for this PR. It's simpler and including a dependency of a JSON parser is not a big deal nowadays IMO, even in Lambda.

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.

3 participants