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

DataQueue is missing the read(n) method (and some others) #81

Closed
schmitch opened this issue Jun 23, 2014 · 20 comments
Closed

DataQueue is missing the read(n) method (and some others) #81

schmitch opened this issue Jun 23, 2014 · 20 comments
Labels

Comments

@schmitch
Copy link

Currently the python spec says that there are several utilities methods that needs to be implemented: http://legacy.python.org/dev/peps/pep-3156/#servers

One is really important the read(n) method. Also two other methods are missing.
readline() and readexcatly(n) if aiohttp won't cover these method it isn't compatible to 3156 and won't work with a lot of projects.

Edit: Here is the class that misses these methods: https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/parsers.py#L281

@fafhrd91
Copy link
Member

Well, aiohttp is not fully wsgi compliant server and it won't. Wsgi spec doesn't say anything about asyncio coroutines. It is possible to make it compliant with readpayload=true parameter, but this is not for production, it reads payload into memory

@schmitch
Copy link
Author

I thought it is compatible with pep-3156? But it isn't.
It still says:

http client/server for asyncio (PEP-3156)

@fafhrd91
Copy link
Member

Ah, sorry. I thought about wsgi specs.

Intention for DataQueue is different from StreamReader. You can wrap payload into StreamReader very easily.

@fafhrd91
Copy link
Member

Regarding compatibility with pep-3156, it says nothing about http. That was one of the reasons why we moved http code from asyncio to separate project.

@schmitch
Copy link
Author

I could, but if I wrap them I would need to write my own program.
But what happens if I use gunicorn with django? It will fail horribly.
So only gunicorn could fix this up, or I would need to write my own 'gunicorn'. Best would be to just include that.

Also PEP-3156 says something about http.

A few functions and classes are provided to simplify the writing of basic stream-based clients and servers, such as FTP or HTTP. Thes are:

It is an Interface and Interfaces should be implemented in my eyes. But who cares.

@fafhrd91
Copy link
Member

gjango will fail horribly, because it is doesn't work with asyncio. Asyncio doesn't work like gevent, you can not just install aiohttp and make your application asynchronous. For non trivial application you have to rewrite it to use asyncio.

I don't understated "Interface" part of your comment. Could you elaborate.

@fafhrd91
Copy link
Member

Won't fix

@benoitc
Copy link

benoitc commented Jun 23, 2014

@fafhrd91 not sure but why a buffered stream reader could not be used there? if buffer is empty or < n it could eventually claim for more data until eof.

@fafhrd91
Copy link
Member

DataQueue is not a bytes stream, it just holds arbitrary objects that parser emits. for example WebSocketParser emits aiohttp.websocket.Message named tuples. if application requires it is very easy to wrap DataQueue into StreamReader.

@benoitc
Copy link

benoitc commented Jun 23, 2014

@fafhrd91 if it can be done easily, i think it would be good to have it imo :)

@fafhrd91
Copy link
Member

sure, if somebody explain how i can do byte streaming on arbitrary objects, like tuples :)

@ludovic-gasc
Copy link
Contributor

From my experience, you can't access to POST variables in a Flask application without a stacktrace with Gunicorn.
No idea to resolve that, but it looses the interest if we can't use main WSGI frameworks with that.

@fafhrd91
Copy link
Member

there is a way. we can change "readpayload" to True, then wsgi.input will work as expected. but this will mean that payload gets loaded into memory.

@asvetlov @benoitc thoughts?

@fafhrd91 fafhrd91 reopened this Jun 23, 2014
@ludovic-gasc
Copy link
Contributor

@benoitc how others workers in Gunicorn work to read payload? If they also load all in memory, "readpayload" to True is acceptable.

@benoitc
Copy link

benoitc commented Jun 24, 2014

@GMLudo sync, thread and the default gevent/eventlet workers are streaming the data. they don't keep the full body in memory.

@fafhrd91
Copy link
Member

readpayload is to True in gaiohttp worker.

@benoitc
Copy link

benoitc commented Jun 28, 2014

well I still think there should be a way to stream it. Should it be another ticket?

@fafhrd91
Copy link
Member

i've just created #87

@benoitc
Copy link

benoitc commented Jun 28, 2014

Thanks!

On Sat, Jun 28, 2014 at 8:23 PM, Nikolay Kim notifications@github.com
wrote:

i've just created #87 #87


Reply to this email directly or view it on GitHub
#81 (comment).

@lock
Copy link

lock bot commented Oct 29, 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 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants