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

WIP: ASGI Integration #1265

Closed
wants to merge 2 commits into from
Closed

WIP: ASGI Integration #1265

wants to merge 2 commits into from

Conversation

tomchristie
Copy link
Contributor

Quick proof-of-concept for adding ASGI support to Sanic.
Refs. #761

Makes the following possible:

from sanic import Sanic
from sanic.response import json

app = Sanic()

@app.route('/')
async def test(request):
    return json({'hello': 'world'})

Then:

uvicorn example:app  # Or `daphne` or `hypercorn`.

I've just added this in as dumb-as-possible way for now, as a proof-of-concept.

  • Doesn't currently handle streaming requests, always just reads body to memory.
  • Doesn't currently handle streaming responses.
  • Doesn't handle websockets.
  • Doesn't currently interact properly with Sanic's exception handling.
  • Naively drops the gunicorn callable, in favor of an ASGI callable.

Note that Ctrl-C to quit may not work properly here, I think due to not having plugged Sanic's exception handling in properly yet.

A more committed approach to moving to ASGI might instead introduce changes to the request/response classes, rather than just adding a dumb shim over the top of Sanic's existing implementation. Additionally it might also adapt the server implementation slightly so that
it actually calls into Sanic applications using ASGI.

Some benefits from moving to ASGI would be:

  • HTTP 2 support. (Currently available in Hypercorn, Daphne. Planned for Uvicorn)
  • PyPy support. (Currently available in Uvicorn and Daphne)
  • Windows support. (Currently available in Daphne. Very nearly available in Uvicorn.)
  • Implementations that fix write-buffer flow control, eg Architectural problems of Sanic framework #1176. (In this PR always send back the response using an asynchronous API, allowing server implementations to pause while the write buffer is flushed, if needed, which I know Uvicorn and Daphne both do, and I expect Quart also does.)
  • Enable a shared ecosystem of ASGI middleware. (Across APIStar, Sanic, Quart, Starlette for starters.)
  • Interoperable server implementations. (Allow folks to develop more robust or faster server implementations, and have that work shared across all ASGI frameworks.)

This PR is more to start a conversation than anything else, and to demonstrate that Sanic's close enough already that moving to ASGI might not necessarily require a massive set of changes.

Obviously I personally think there'd be a big long-term benefit to Sanic moving to an ASGI interface, both for Sanic itself, and for the community as a whole. Tho this PR is more of a proof-of-concept at this stage, than a serious attempt.

@skewty
Copy link

skewty commented Sep 24, 2018

This seems stalled again. Is this still a priority?

This along with Gino makes sanic quite appealing for a project we just got approval for at work. Without working web sockets, it is a non starter for us (unless I am understanding things incorrectly).

@tomchristie
Copy link
Contributor Author

Sanic has working websocket support, so I'm not sure if this issue is relevant to you or not.
Personally I'm up for helping progressing this issue myself, but I don't know what the sanic team's stance on it is.

@ahopkins
Copy link
Member

I think this is something that is definitely on the radar. It makes sense that if asgi is going to have a broader applicability and become adopted outside django-channels (for example), then we should look into applying it here. The point about a benefit for both Sanic and the Python community is a motivating factor.

As said above, websockets are a part of sanic now. So, if that is what you need to move forward, you should be able to.

Right now the Sanic team is doing a little reorganizing to shift to being a community run organization. Part of the discussions have included placing a greater emphasis on websockets.

That, plus this being a "proof of concept" PR puts a little pause on this before we would merge this asgi support PR. @tomchristie graciously put some effort into making this work inside Sanic, and I do believe this (or something that achieves this) will be adopted.

@tomchristie
Copy link
Contributor Author

All sounds super positive! Up for being involved/helping out as needed. Cheers!

@sjsadowski
Copy link
Contributor

@tomchristie can you update your fork so we can get the tests to run again against current master?

@sjsadowski sjsadowski modified the milestones: Future Release, 19.03 Sep 27, 2018
@sjsadowski
Copy link
Contributor

Tagged this for inclusion for 19.03

@sjsadowski sjsadowski added this to In Progress in 19.3 Oct 18, 2018
@sjsadowski sjsadowski moved this from In Progress to To Do in 19.3 Oct 18, 2018
@tomchristie
Copy link
Contributor Author

Discussion: https://community.sanicframework.org/t/considering-asgi-support/70

@ahopkins ahopkins changed the title ASGI Proof-of-concept WIP: ASGI Integration Nov 1, 2018
@ahopkins ahopkins mentioned this pull request Jan 13, 2019
@sjsadowski sjsadowski closed this Mar 3, 2019
@sjsadowski sjsadowski reopened this Mar 3, 2019
@sjsadowski sjsadowski modified the milestones: 19.3, 19.6 Mar 3, 2019
@sjsadowski sjsadowski removed this from To Do in 19.3 Mar 3, 2019
@sjsadowski sjsadowski added this to To Do in 19.6 Mar 3, 2019
@ahopkins
Copy link
Member

ahopkins commented Apr 7, 2019

We can leave this one closed in favor of #1475. I am going to start working on the test client issue.

@ahopkins ahopkins closed this Apr 7, 2019
19.6 automation moved this from To Do to Done Apr 7, 2019
@ahopkins ahopkins removed this from Done in 19.6 May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants