🚧 Add event source detection

[ediskandarov-cur]

• add event source types
• add event source detection
• use event source type in scope creation

This PR is a one-step closet to support different event sources.

Also, we should construct responses according to the request type, that work is out of the PR scope though.

[jordaneremieff]

Hi @emcpow2, thanks for the PR, but I'm not sure if this approach provides any significant advantage to what we have currently.

This PR is a one-step closet to support different event sources.

Could you go into more detail about what you mean here?

[ediskandarov-cur]

Hey @jordaneremieff ,

The main purpose is to further precisely construct response objects.
(it's hard to follow response structure in the current implementation, but I'm sure it is not 100% compliant with some event sources).

Response structure differs with depends on the request event.

Response for ALB request with multivalue headers disabled

{
    "isBase64Encoded": false,
    "statusCode": 200,
    "statusDescription": "200 OK",
    "headers": {
        "Set-cookie": "cookies",
        "Content-Type": "application/json"
    },
    "body": "Hello from Lambda (optional)"
}

Response for ALB request with multivalue headers enabled

{
    "isBase64Encoded": false,
    "statusCode": 200,
    "statusDescription": "200 OK",
    "multiValueHeaders": {
        "Set-cookie": ["cookie-name=cookie-value;Domain=myweb.com;Secure;HttpOnly","cookie-name=cookie-value;Expires=May 8, 2019"],
        "Content-Type": ["application/json"]
    },
    "body": "Hello from Lambda (optional)"
}

Response for API Gateway Payload v1 request

{
    "isBase64Encoded": false,
    "statusCode": 200,
    "headers": {"headerName": "headerValue"},
    "multiValueHeaders": {"headerName": ["headerValue", "headerValue2"]},
    "body": "Hello from Lambda!"
}

Response for API Gateway Payload v2 request

{
    "cookies" : ["cookie1", "cookie2"],
    "isBase64Encoded": false,
    "statusCode": 200,
    "headers": { "headerName": "headerValue"},
    "body": "Hello from Lambda!"
}

References

• Application Load Balancers - Lambda functions as targets
• Working with AWS Lambda proxy integrations for HTTP APIs

[jordaneremieff]

@koxudaxi could you take a look at this one and share your thoughts when you can?

[koxudaxi]

@emcpow2
I could understand why did you create this PR.

However, I don't know the implementation is the best approach. 🤔
Because I can't look next phase which is you said out of scope the PR.

Also, we should construct responses according to the request type, that work is out of the PR scope though.

Could you explain the next phase design?

[ediskandarov-cur]

This is how I see it

Long term outcomes:

• Mangum class communicate to cloud - understand cloud events and builds corresponding responses.
• Mangum class does not know ASGI details
• We introduce MangumRequest and MangumResponse classes
• Mangum class send MangumRequest and receive MangumResponse messages to HTTPCycle
• HTTPCycle translate MangumRequest to ASGI request and communicate to application
• In response HTTPCycle sends MangumResponse

That's a way we could build an extendible and cloud agnostic Lambda ASGI adapter.

Feedback is welcome. I recognise I could overlook pitfalls and implementation details.

Talking about the next step:

1. introduce MangumRequest and MangumResponse
2. move scope construction to HTTPCycle and utilise MangumRequest and MangumResponse
3. in mangum class create a mapping for request event type and mangum response

[koxudaxi]

@emcpow2
Thank you for the detail of the design.
I'm afraid to increase the execution time of mangum.
Everyone hates it to save money and performance. So, I asked you for the detail.
Also, a Complex structure becomes the wall to contribute to the project.
We need to choose Pros and Cons 🤔

What did you think about it?

[jurasofish]

For what it's worth I think this PR looks like a step in the right direction - it clearly separates out the detection of the event source type into a nice neat element which can be reasoned about, and tested, in isolation. Construction of response based on event source type as you've suggested is a good idea too for the future.

I don't see how this PR could have any tangible impact on performance.
Looking at complexity - I think in the long term, if more event sources are added, this approach will be less complex.
Overall it's just good design.

[jordaneremieff]

So, I've previously experimented with a few different approaches towards a similar goal, but the correct approach I think depends mostly on whether or not we want to include multi-cloud support or focus exclusively on AWS and pursue greater support for AWS features (e.g. websockets, etc.).

I guess we can probably design something that accommodates multiple use-cases, but I want to be careful about the foundational pieces here because event handlers arguments and types can vary across platforms and even AWS alone has a lot of variety in the request/response structures. If we do something like this, then I think instead of additional MangumRequest and MangumResponse classes that we instead only modify the existing HTTPCycle class to make it more generic or easier to extend.

[ediskandarov-cur]

I don't see a huge difference between various types of AWS events and an event from Google/Azure.

Let's clearly state that Mangum does support a subset of cloud events, but does it in a consistent way?

For instance, if AWS ALB support is declared - then there's a guarantee of 100% compliance with specs.

---

In order to move further, we need to outline class responsibilities.

So that we do not mess abstractions and have a clear design.

Are these correct?

• Mangum(Creates an adapter instance.)
• HTTPCycle(Manages the application cycle for an ASGI http connection.)

If there's an agreement to support various types of events - whose responsibility to maintain cloud-agnostic adopters?

Possible solutions:

• Create a subclass of Mangum for each of Event types(MangumAWSALB, MangumAWSAPIGB, etc.); in this case, we shall extract response serialization from HTTPCycle
• Perform event type detection and transformation in Mangum class(this PR)

[jordaneremieff]

I don't see a huge difference between various types of AWS events and an event from Google/Azure.

I consider the differences significant. For example, I've created similar projects for both in the past, and there is some nuance to how the request/responses need to be handled for both GCF and Azure. From what I recall, Google Cloud Functions use the Flask request class for events and expects either a Flask Response object, response body, or tuple to be returned. In the case of Azure, it relies on specific HTTP request and response classes from its own Python worker library, and the name of the parameter provided for the request event object can be configurable.

I originally supported Azure in this project and had considered multi-cloud support at various stages, but the latest discussion around this was in this issue where I ultimately decided to focus on supporting just AWS and instead opened an issue in the official Azure repository to support ASGI (similar to how it supports WSGI).

Based on an earlier comment:

(it's hard to follow response structure in the current implementation, but I'm sure it is not 100% compliant with some event sources).

It seems like the main problem currently is with understanding the response structure and making it compliant for the recently added ALB events? In this case, the PR should focus on determining where the response is non-compliant and difficult to understand without any consideration for potential use-cases for other cloud providers because supporting multiple cloud providers isn't currently in the project roadmap - to pursue that aspect further should be done in a new issue on the tracker for discussion and agreement first.

[ediskandarov-cur]

@jordaneremieff , thanks for the clarifications.

The reason I tried to contribute was to build something on top of mangum and add missing features to the project, and as a side effect maximizing positive impact to mangum.

Fixing solely ALB issue is not my priority.

I'm stepping out for now.