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

Streaming Server #1876

Merged
merged 104 commits into from Jan 10, 2021
Merged

Streaming Server #1876

merged 104 commits into from Jan 10, 2021

Conversation

Tronic
Copy link
Member

@Tronic Tronic commented Jun 21, 2020

Moved from #1791

A plain Python reimplementation of Sanic's built-in server and related restructuring also in ASGI backend and in response handling. Includes first-class support for streaming, improved performance and better ground for further development.

By the sheer volume of it, it also brings some breaking changes like the removal of custom protocol support but typical applications that do not do anything special with the server's internals should remain mostly unaffected.

Installation:

pip install git+https://github.com/huge-success/sanic@streaming

Issues of current git master, resolved in this:
Fix #1653
Fix #1722
Fix #1730
Fix #1749
Fix #1785
Fix #1790
Fix #1804
Fix #1824
Fix #1875
Fix #1988

…nger be supported. Chunked mode is now autodetected, so do not put content-length header if chunked mode is preferred.
…), all requests made streaming. A few compatibility issues and a lot of cleanup to be done remain, 16 tests failing.
…timeout=<value> which wasn't the case with existing implementation, and secondly none of the other web servers I tried include this header.
…led. Rewritten using receive_body and case switching to make it fail if bypassed.
@ahopkins ahopkins requested review from ashleysommer, sjsadowski, vltr and a team and removed request for sjsadowski December 30, 2020 09:32
@ahopkins
Copy link
Member

@huge-success/sanic-core-devs Please take a look at this one. I'd like to get this merged by the end of the weekend. Even if it is not 100% perfect and release worthy, I want to get this merged because of the large number of changes that it entails. Having it merged into master will allow us to continue making other PRs without resolving too many conflicts. Release it not set until March, but I want to make sure there is plenty of time to test this in master.

@ahopkins ahopkins added this to the v21.3 milestone Dec 30, 2020
@ahopkins ahopkins mentioned this pull request Dec 30, 2020
@ahopkins
Copy link
Member

This pull request has been mentioned on Sanic Community Discussion. There might be relevant details there:

https://community.sanicframework.org/t/new-streaming-api-and-other-major-changes/518/15

@ahopkins
Copy link
Member

ahopkins commented Jan 5, 2021

This pull request has been mentioned on Sanic Community Discussion. There might be relevant details there:

https://community.sanicframework.org/t/2021-release-managers-and-updates/781/1

@ahopkins ahopkins removed the on hold label Jan 5, 2021
Copy link
Member

@ahopkins ahopkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll handle these. When done, I want to merge. @sanic-org/sanic-core-devs any other thoughts before I do so?

sanic/asgi.py Outdated Show resolved Hide resolved
sanic/request.py Show resolved Hide resolved
sanic/response.py Show resolved Hide resolved
sanic/response.py Show resolved Hide resolved
sanic/response.py Outdated Show resolved Hide resolved
@ahopkins ahopkins self-assigned this Jan 10, 2021
@ahopkins ahopkins merged commit 7028eae into master Jan 10, 2021
@ahopkins ahopkins deleted the streaming branch January 10, 2021 22:45
@ashleysommer
Copy link
Member

ashleysommer commented Jan 19, 2021

Just looking over these merged changes now, to learn how Sanic works at a low level now.

I have noticed it looks like there is a possibility for the Request Middleware to run twice on a request.
Both the app.handle_request and app.handle_exception methods execute the Request middleware.

If the main handler encounters an exception after running the middleware, then the exception handler will execute and run the request middleware again.

Edit:
Perhaps request.request_middleware_started is already used to mitigate against that?

@ahopkins
Copy link
Member

Yeah. That is the purpose of it. I think eventually I want to get rid of it and not have to carry that state trigger (there is not a similar one for response).

@ahopkins
Copy link
Member

ahopkins commented Feb 9, 2021

This pull request has been mentioned on Sanic Community Discussion. There might be relevant details there:

https://community.sanicframework.org/t/highly-different-cpu-usage-when-proxying-m2tp-udp-unicast-stream-to-a-streamed-sanic-response/795/10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment