search

aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
14.99k stars 2k forks source link

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

Closed schmitch closed 10 years ago

schmitch commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

http client/server for asyncio (PEP-3156)

fafhrd91 commented 10 years ago

Ah, sorry. I thought about wsgi specs.

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

fafhrd91 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

Won't fix

benoitc commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

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

fafhrd91 commented 10 years ago

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

ludovic-gasc commented 10 years ago

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 commented 10 years ago

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?

ludovic-gasc commented 10 years ago

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

benoitc commented 10 years ago

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

fafhrd91 commented 10 years ago

readpayload is to True in gaiohttp worker.

benoitc commented 10 years ago

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

fafhrd91 commented 10 years ago

i've just created #87

benoitc commented 10 years ago

Thanks!

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

i've just created #87 https://github.com/KeepSafe/aiohttp/issues/87

— Reply to this email directly or view it on GitHub https://github.com/KeepSafe/aiohttp/issues/81#issuecomment-47434721.

lock[bot] commented 4 years ago

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.