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

Feature request: Allow the ApiGatewayResolver to support an existing subclass of BaseProxyEvent #3267

Closed
1 of 2 tasks
BVMiko opened this issue Oct 29, 2023 · 7 comments
Closed
1 of 2 tasks
Labels
feature-request feature request

Comments

@BVMiko
Copy link
Contributor

BVMiko commented Oct 29, 2023

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:

@event_source(data_class=APIGatewayProxyEvent)
def lambda_handler(event: APIGatewayProxyEvent, context: LambdaContext):
    logger.append_keys(user_agent=event.get_header_value("user-agent", "[No User-Agent]"))
    return app.resolve(event.raw_event, context)

Where I feel I should just be able to pass in the instance which was already created:

    return app.resolve(event, context)

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

@event_source(data_class=APIGatewayProxyEvent)
def lambda_handler(event: APIGatewayProxyEvent, context: LambdaContext):
    logger.append_keys(user_agent=event.get_header_value("user-agent", "[No User-Agent]"))
    return app.resolve(event.raw_event, context)

becomes

@event_source(data_class=APIGatewayProxyEvent)
def lambda_handler(event: APIGatewayProxyEvent, context: LambdaContext):
    logger.append_keys(user_agent=event.get_header_value("user-agent", "[No User-Agent]"))
    return app.resolve(event, context)

Also it allows a custom ProxyEvent to be used such as:

class CustomProxyEvent(BaseProxyEvent):
    def __init__(self, data: dict[str, Any], json_deserializer: Optional[Callable]=None):
        data_wrapper = {"path": "", "httpMethod": data["action"], "body": json.dumps(data)}
        super().__init__(data_wrapper, json_deserializer)
        self._json_data = data

    @property
    def action(self) -> str:
        return cast(str, cast(dict, self._json_data)["action"])

    @property
    def content(self) -> str:
        return cast(str, cast(dict, self._json_data)["content"])

    # Unfortunately this must still be provided unless we mark it as optional.
    # I have a separate commit prepared, if you are willing to consider I can include it
    def header_serializer(self) -> BaseHeadersSerializer:
        return MultiValueHeadersSerializer()

app = ApiGatewayResolver(debug=True)
logger = Logger()

@app.route("", "foo")
def group_stats_get() -> Response:
    return Response(200, body=app.current_event.content)

@event_source(data_class=CustomProxyEvent)
def handler(event: CustomProxyEvent, context: LambdaContext):
    logger.append_keys(action=event.action)
    return app.resolve(event, context)

with the data

{"action": "foo", "content": "bar"}

Alternative solutions

No response

Acknowledgment

@leandrodamascena
Copy link
Contributor

Hey @BVMiko! Thanks for opening this issue and send the PR! We will review this week.

@leandrodamascena leandrodamascena moved this from Triage to Pending review in Powertools for AWS Lambda (Python) Oct 31, 2023
@leandrodamascena leandrodamascena removed the triage Pending triage from maintainers label Oct 31, 2023
@leandrodamascena leandrodamascena self-assigned this Oct 31, 2023
@heitorlessa heitorlessa moved this from Pending review to Triage in Powertools for AWS Lambda (Python) Dec 6, 2023
@heitorlessa
Copy link
Contributor

Adding this back as Triage as we got sidetracked with re:Invent and operational issues

@BVMiko
Copy link
Contributor Author

BVMiko commented Feb 6, 2024

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:

  • The ability to combine the @event_source() decorator with the ApiGatewayResolver
  • The ability to use a custom proxy event for input processing

It would also make sense to make the header_serializer() an optional method on subclasses of BaseProxyEvent, any resulting proxy event class implementations could be simplified.

@heitorlessa
Copy link
Contributor

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 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 heitorlessa moved this from Triage to Pending customer in Powertools for AWS Lambda (Python) Feb 20, 2024
@heitorlessa
Copy link
Contributor

Reiterating this idea from a new issue wanting Lex's support #3807

@leandrodamascena
Copy link
Contributor

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 /bucket/file_x and /bucket/file_y, the experience could be:

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.

@github-project-automation github-project-automation bot moved this from Pending customer to Coming soon in Powertools for AWS Lambda (Python) Jan 24, 2025
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@leandrodamascena leandrodamascena moved this from Coming soon to Closed in Powertools for AWS Lambda (Python) Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request feature request
Projects
3 participants