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

Allow ClientResponse to define custom flow control #100

Closed
wants to merge 1 commit into from
Closed

Allow ClientResponse to define custom flow control #100

wants to merge 1 commit into from

Conversation

kxepal
Copy link
Member

@kxepal kxepal commented Jul 6, 2014

Also make DataQueue act as like as StreamReader to let FlowControlStreamReader get able to be replaced by FlowControlDataQueue preserving the common behaviour.

Also make DataQueue act as like as StreamReader to let
FlowControlStreamReader get able to be replaced by FlowControlDataQueue
preserving the common behaviour.
@asvetlov
Copy link
Member

asvetlov commented Jul 6, 2014

LGTM

@fafhrd91
Copy link
Member

fafhrd91 commented Jul 6, 2014

Idea for DataQueue that it can hold any value from parser. and None of empty bytes string are valid values. and with this change i need to treat empty bytes specially as it is not part of what parser emits. even if we decide to accept this change we cannot just return empty bytes, we have define sentinel const and then expose as public api, etc.

example is websockets, servers it receives aiohttp.websockets.Message objects.
https://github.com/KeepSafe/aiohttp/blob/master/examples/wssrv.py#L63

so my opinion is -1

@fafhrd91 fafhrd91 closed this Jul 6, 2014
@fafhrd91
Copy link
Member

fafhrd91 commented Jul 6, 2014

please create PR against master.

just read your comment on 87, we can add special DataQueue that works with bytes objects. and it can inherit from FlowControlDataQueue and it can return b"" instead of EofStream.

@kxepal
Copy link
Member Author

kxepal commented Jul 7, 2014

Ah, I didn't mind another cases when DataQueue may be used. Agreed for having special class to handle this case.

@lock
Copy link

lock bot commented Oct 30, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants