-
Notifications
You must be signed in to change notification settings - Fork 406
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
Feature request: Allow the ApiGatewayResolver to support an existing subclass of BaseProxyEvent #3267
Comments
Hey @BVMiko! Thanks for opening this issue and send the PR! We will review this week. |
Adding this back as Triage as we got sidetracked with re:Invent and operational issues |
Hey, I just wanted to ping and see if anyone had chance to check this issue and corresponding PR yet. The current code's limitation doesn't seem to be necessary; and this seems like a pretty safe enhancement to accept any subclass of a proxy event. I've been using a fork with this commit for several projects over the last few months and it's been handy for several reasons:
It would also make sense to make the |
I replied in the PR back then but didn't copy it here, sorry, pasting verbatim. hey @BVMiko apologies on behalf of the team, we dropped the ball in coming back with a timely counter-offer for this extension. How about we work on a RFC to make it possible to route anything with a generic Event Handler? While this applies to you, we already had customers telling us in private that they'd love an Event Handler for EventBridge, S3, Step Functions, SNS, etc. It'd be great to have a way to (1) specify which key(s) of the event to do the routing, and (2) declare the shape of the event available in That way, we can have a dedicated documentation for this, and future-proof new Event Handler specializations as it gets popular, just like we have REST and GraphQL Event Handlers today. What do you say? |
Reiterating this idea from a new issue wanting Lex's support #3807 |
Hi @BVMiko and everyone! I was reading this issue and PR #3268 and realized that we have a great opportunity to finally create a new AsyncResolver/GenericResolver/EventSourceMapResolver resolvers and provide a great experience for customers. While I see value in working to merge PR #3268, I'm not sure this is the best strategy because we will force customers to create their own classes and possibly have difficulties with some field definitions and such. One of the ideas for Powertools this year is to create this new Resolver. We know that customers may want to use the same Lambda to process different S3 events, imagine for example customers triggering S3 events for different prefixes like from aws_lambda_powertools.async_resolver import S3AsyncResolver
app = S3AsyncResolver()
@app.prefix(path='/bucket/file_x'):
def process_file_x():
print(app.context.etag)
# do stuff for file_x
...
@app.prefix(path='/bucket/file_y'):
def process_file_x():
print(app.context.size)
# do stuff for file_y
... Even the example if very specific for S3, it can be used with EventBridge, SQS, SNS and many other services. We already have a PR to implement this, but it's need a RFC to agree with the API definition, edge cases, best developer experience and others. I left this comment and hope to have this RFC by the first week of February. I'm closing this issue and the PR #3268 and will keep you posted once we have the RFC. Thanks for always bringing great ideas to improve the Powertools experience for developers. |
|
Use case
I often want to retrieve values from a ProxyEvent using the helper functions prior to the
app.resolve()
call; and find myself creating a second instance for it, for example:Where I feel I should just be able to pass in the instance which was already created:
I've now found myself wanting to use the ApiGatewayResolver class with a Lambda triggered by EventBridge Rules, and found that it can be done trivially with a custom ProxyEvent subclass -- but only if the ApiGatewayResolver accepts the custom ProxyEvent.
The changes required to support a custom ProxyEvent are very minimal and shouldn't be backwards-incompatible.
I see there was an adjustment about 18 months ago in #1152 & #1159 where a warning message and fallback were added; my proposed change involves replacing the warning with fully supporting the existing ProxyEvent class.
Solution/User Experience
becomes
Also it allows a custom ProxyEvent to be used such as:
with the data
Alternative solutions
No response
Acknowledgment
The text was updated successfully, but these errors were encountered: