Kludex / mangum

AWS Lambda support for ASGI applications
http://mangum.fastapiexpert.com/
MIT License
1.73k stars 126 forks source link

AWS WebSocket support #132

Closed jordaneremieff closed 3 years ago

jordaneremieff commented 4 years ago

I've removed the existing WebSocket support as part of https://github.com/jordaneremieff/mangum/issues/127 with the intention of developing it outside of this package, then later migrating it back in (if it makes sense to do so). My main concern is that I want to avoid impacting the stability of the HTTP implementation which is the primary use-case, and the maintenance burden was too much for the amount of capacity I have to spare currently. I also want to re-design it with broadcast support as an initial requirement rather than a feature added on later.

I think a new project might use Mangum as a dependency, or could end up looking something like this:

from mangum import Mangum
from mangum_ws import MangumWebSocket

def handler(event, context):
    if 'eventType' in event['requestContext']:
        asgi_handler = MangumWebSocket(app)
    else:
        asgi_handler = Mangum(app)

    return asgi_handler(event, context)

Not really sure. I haven't been motivated to take this up for awhile, but if anyone wants to give it a shot I can help where possible.

jordaneremieff commented 4 years ago

The next large item I'm interested in working on is https://github.com/jordaneremieff/mangum/issues/86, so I won't likely revisit this for awhile. Still happy to assist anyone that wants to take this on, but I'll be focusing on supporting multiple FaaS platforms before doing anything further myself.

satyajitghana commented 3 years ago

any update/progress on this ? i've been wanting to use websockets with lambda using mangum

jordaneremieff commented 3 years ago

@satyajitghana I've note had a chance to come back around to this and likely won't for awhile, but I was thinking about how to accommodate a plugin system to more easily integrate potential solutions for this kind of thing. Once I have made any progress here I'll update the ticket.

ptrhck commented 3 years ago

I am also interested and would like playing around with an older version. But where can I find the documentation for older versions that include the websockets info?

satyajitghana commented 3 years ago

@ptrhck 0.9.0 had websocket support, https://github.com/jordaneremieff/mangum/tree/8de9e84bfa22f1fa45ae601b781d9f0374b01dd9 you can find the docs there

ptrhck commented 3 years ago

Thanks! I have also found the commit before websocket was removed.

Very unfortunate that it has been removed but hopefully there will be a new project soon :-)

villekr commented 3 years ago

I would be interested to look at this in relation with https://github.com/mobilityhouse/ocpp/issues/168. However AWS API Gateway WebSockets API is not primary focus due to it's incompatibility with ocpp-j endpoint format requirements. So instead custom websockets gateway will be used but the idea would be similar as with AWS API Gateway. Websocket broadcast support however is not in my interest.

I briefly checked the removed websockets related code and it looks good. Was there some major problems or missing things in the implementation that I should be aware of?

Basically I like idea having Mangum and MangumWebSocket being distinct classes and even so that one should explicitly define which one to use instead of dynamically instantiating either one based on incoming event's attributes.

Also implementation behind post_to_connection should be changeable to facilitate different websocket callback apis.

jordaneremieff commented 3 years ago

@villekr

I briefly checked the removed websockets related code and it looks good. Was there some major problems or missing things in the implementation that I should be aware of?

No, there weren't any specific issues in the implementation. My primary reason for removing it was that I did not have the capacity to properly support the pattern I ended up using, however, I think it is the correct solution for this problem generally. I agree a separate class would be better than trying to determine this based on the incoming event, though I haven't spent time to think about the specifics of how to implement this yet.

jordaneremieff commented 3 years ago

Given the changes in https://github.com/jordaneremieff/mangum/pull/170 I think we could potentially re-add WebSockets to this project using the new pattern for implementing handlers.

Probably something like

        if (
            "requestContext" in trigger_event
            and "routeKey" in trigger_event["requestContext"]
        ):
            from . import AwsWsGateway

            return AwsWsGateway(trigger_event, trigger_context, **kwargs)

as the entrypoint for the event handling, then what's in https://github.com/jordaneremieff/mangum/tree/broadcast can be refactored into the new event workflow.

eduardovra commented 3 years ago

Is anyone working on this? It's is a nice feature and I have an interest in helping out. I can provide an initial PR and upon @jordaneremieff validation, we iterate to the final solution. What do you think?

koxudaxi commented 3 years ago

@eduardovra Thank you for your great suggestion :tada: I have not worked on the feature, because I have not got the time :cry:

If you create the PR following @jordaneremieff's idea then we will review it :+1:

eduardovra commented 3 years ago

Hi, I've created the draft PR (#190) and left a few questions. This is my first attempt to contribute to the project and any help will be much appreciated.

techish commented 3 years ago

@jordaneremieff when will @eduardovra's PR be accepted? I am desperately waiting for this as I need to run my Django application with websocket endpoints on AWS Lambda and API Gateway.

jordaneremieff commented 3 years ago

@techish I have some review comments on that PR to be resolved then I will merge. The only thing really missing atm is a minimal example that can be included in documentation that I can test in a deployment before I merge, though I might just push this myself when I have time later if becomes a potential blocker to merging the changes.

ptrhck commented 3 years ago

@jordaneremieff when will @eduardovra's PR be accepted? I am desperately waiting for this as I need to run my Django application with websocket endpoints on AWS Lambda and API Gateway.

Once merged, could you share some example code how you have deployed Django? I am very interested in following this approach and will contribute if I can!

jordaneremieff commented 3 years ago

This has been merged (thanks @eduardovra!), though I need to do a few things to prepare for the release and will close this ticket once it's out (probably later this week). In the meantime, if anyone decides to try this out directly from the main branch and encounters any problems please report back on this issue.

jordaneremieff commented 3 years ago

This has been released in 0.12.0. Please open a new issue for further discussion/issue reports.