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

feat(event_handler): Allow the ApiGatewayResolver to support an existing subclass of BaseProxyEvent #3268

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from

Conversation

BVMiko
Copy link
Contributor

@BVMiko BVMiko commented Oct 29, 2023

Issue number: #3267

Summary

Changes

Allow the ApiGatewayResolver to support an existing subclass of BaseProxyEvent. The existing subclass can be a custom implementation, and allows overriding the route matching logic

User experience

Examples were posted in the linked issue #3267

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@BVMiko BVMiko requested a review from a team as a code owner October 29, 2023 05:05
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 29, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
1.2% 1.2% Duplication

@heitorlessa
Copy link
Contributor

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 current_event.

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?

@heitorlessa
Copy link
Contributor

Bringing this up to your inbox in case you missed it - would love to hear your thoughts

@Tankanow
Copy link
Contributor

Tankanow commented May 6, 2024

@heitorlessa, is there any movement on the Generic Router? I would love this feature for all the reasons you've mentioned in other comments. I'm happy to help out if needed.

@heitorlessa
Copy link
Contributor

heitorlessa commented May 7, 2024

@heitorlessa, is there any movement on the Generic Router? I would love this feature for all the reasons you've mentioned in other comments. I'm happy to help out if needed.

sadly no, we're short staffed so we'd appreciate any help -- I'd start with a RFC to account for 1/ ability to select multiple fields for routing an event to a function, 2/ generics to allow any data class/model to be used for app.current_event, 3/ ability to have middlewares.

More than happy for this to be a completely new thing as we split off from the single file issue of Event Handler in v3.

@Tankanow
Copy link
Contributor

Tankanow commented May 7, 2024

@heitorlessa, is there any movement on the Generic Router? I would love this feature for all the reasons you've mentioned in other comments. I'm happy to help out if needed.

sadly no, we're short staffed so we'd appreciate any help -- I'd start with a RFC to account for 1/ ability to select multiple fields for routing an event to a function, 2/ generics to allow any data class/model to be used for app.current_event, 3/ ability to have middlewares.

More than happy for this to be a completely new thing as we split off from the single file issue of Event Handler in v3.

@heitorlessa, this sounds like a great start. I'll start working on an RFC.

@leandrodamascena
Copy link
Contributor

Hi @Tankanow! How is it going? I'd like to see if you have any updates to share regarding this new RFC.

Thanks!

@Tankanow
Copy link
Contributor

Hi @leandrodamascena,

Thanks for the ping. I just got back from a long vacation. I'm going to start work on it this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge event_handlers size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Allow the ApiGatewayResolver to support an existing subclass of BaseProxyEvent
4 participants