Pausable response streams #1179
Conversation
|
Tests are failing because new aiohttp v3.1.0 released 7 days ago (https://github.com/aio-libs/aiohttp/releases#v3.1.0) brings in compatibility problems. |
|
Much better but as you've mentioned the change is not backward compatible. I don't know is it ok or not (Sanic is in Beta stage, this assumes some place for breaking changes if needed). |
ashleysommer
force-pushed
the
pausable_response_stream
branch
2 times, most recently
from
75dc05f to
89df1b4
Compare
|
I've rebased this PR onto the current master, so the tests should pass now. |
|
Damn, that hanging bug in the tests from last week is back. |
|
Is there a way to force the travis test to run again, without pushing another change? |
|
There should be a |
|
No, I don't see the "restart build" button because I don't have write access. |
|
this is cool, we may wanna bump up the major version for next release |
|
Ok, well since it's likely a major version bump, are there any other breaking changes we've been wanting to make but holding off? |
|
Any news on this? I would be very happy with a new release :) |
|
cc: @seemethere |
|
@seemethere @yunstanford Any update on getting this merged? |
…or pause_writing and resume_writing. These are needed for the correct functioning of built-in tcp flow-control provided by uvloop and asyncio. This is somewhat of a breaking change, because the `write` function in user streaming callbacks now must be `await`ed. This is necessary because it is possible now that the http protocol may be paused, and any calls to write may need to wait on an async event to be called to become unpaused. Updated examples and tests to reflect this change. This change does not apply to websocket connections. A change to websocket connections may be required to match this change.
ashleysommer
force-pushed
the
pausable_response_stream
branch
from
4931887 to
ec0b99f
Compare
|
I've just rebased this PR to current master HEAD. There wasn't any conflicts, but I didn't want it to drift too far away from current. |
|
@ashleysommer looks like there are some tox/pep8 errors: |
|
@Rofls, those must've been caused by the rebase. I've just fixed the formatting up, should pass now. |
| @@ -83,7 +83,7 @@ def test_request_stream_app(): | |||
| body = await request.stream.get() | |||
| if body is None: | |||
| break | |||
| response.write(body.decode('utf-8')) | |||
| await response.write(body.decode('utf-8')) | |||
There was a problem hiding this comment.
PR looks good. Hoever, won't this require an update to the docs to use the await syntax for streaming?
There was a problem hiding this comment.
Yes. You're right.
I changed the response-streaming example, but I forgot to change the response-streaming docs.
add await syntax to response.write in response-streaming docs.
|
@r0fls |
tests/test_response.py
Outdated
|
|
||
| @streaming_app.listener('after_server_start') | ||
| async def run_stream(app, loop): | ||
| await response.stream() | ||
| assert response.transport.write.call_args_list[1][0][0] == ( | ||
| # assert response.protocol.push_data.call_args_list[1][0][0] == ( |
There was a problem hiding this comment.
these comments seem accidental -- did you mean to leave them here? They seem to be the exact same as the tests themselves
There was a problem hiding this comment.
TBH, I don't remember.
They look accidental. I will remove them.
This commit adds handlers for the asyncio/uvloop protocol callbacks for pause_writing and resume_writing.
These are needed for the correct functioning of built-in tcp flow-control provided by uvloop and asyncio. See #1176
This is somewhat of a breaking change, because the
writefunction in user streaming callbacks now must beawaited.This is necessary because it is possible now that the http protocol may be paused, and any calls to write may need to wait on an async event to be called to become unpaused.
Updated examples and tests to reflect this change.
This change does not apply to websocket connections. A change to websocket connections may be required to match this change.