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

Error serving aiohttp #1338

Closed
diogobaeder opened this Issue Aug 25, 2016 · 11 comments

Comments

@diogobaeder

diogobaeder commented Aug 25, 2016

Hi,

When trying to serve an aiohttp==0.22.5 application with gunicorn==19.6.0 using the gaiohttp worker class (as documented) I can an error:

Traceback (most recent call last):
  File "/<redacted>/lib/python3.5/site-packages/aiohttp/server.py", line 285, in start
    yield from self.handle_request(message, payload)
  File "/<redacted>/lib/python3.5/site-packages/aiohttp/wsgi.py", line 146, in handle_request
    riter = self.wsgi(environ, response.start_response)
TypeError: __call__() takes 1 positional argument but 3 were given

I noticed that Application.__call__() accepts no parameters, but WSGIServerHttpProtocol.handle_request() tries to call it as a WSGI app (with two arguments, environ and start_response). So I believe Application.__call__() could perhaps accept variable parameters and just ignore them, but I'm not sure this would impact anything else.

However, when following the aiohttp documentation on deploying with Gunicorn, that is, using aiohttp.worker.GunicornWebWorker, then it works fine for me.

@berkerpeksag

This comment has been minimized.

Show comment
Hide comment
@berkerpeksag

berkerpeksag Aug 25, 2016

Collaborator

If aiohttp ships with its own worker implementation perhaps we can deprecate ours. Here is the relevant part from http://aiohttp.readthedocs.io/en/stable/gunicorn.html#start-gunicorn:

The custom worker subclass is defined in aiohttp.worker.GunicornWebWorker and should be used instead of the gaiohttp worker provided by Gunicorn, which supports only aiohttp.wsgi applications

Collaborator

berkerpeksag commented Aug 25, 2016

If aiohttp ships with its own worker implementation perhaps we can deprecate ours. Here is the relevant part from http://aiohttp.readthedocs.io/en/stable/gunicorn.html#start-gunicorn:

The custom worker subclass is defined in aiohttp.worker.GunicornWebWorker and should be used instead of the gaiohttp worker provided by Gunicorn, which supports only aiohttp.wsgi applications

@diogobaeder

This comment has been minimized.

Show comment
Hide comment
@diogobaeder

diogobaeder Aug 25, 2016

Sounds good to me. :-)

diogobaeder commented Aug 25, 2016

Sounds good to me. :-)

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Aug 28, 2016

Owner

normally people from aiohttp are supposed to update the worker there. @asvetlov has the commit bit. I am wondering now why the worker hasn't been updated there ....

@asvetlov should we deprecate the worker here? Why things changed?

Owner

benoitc commented Aug 28, 2016

normally people from aiohttp are supposed to update the worker there. @asvetlov has the commit bit. I am wondering now why the worker hasn't been updated there ....

@asvetlov should we deprecate the worker here? Why things changed?

@asvetlov

This comment has been minimized.

Show comment
Hide comment
@asvetlov

asvetlov Aug 28, 2016

Collaborator

Oops. Sorry. I'll update the worker in a few days.

Collaborator

asvetlov commented Aug 28, 2016

Oops. Sorry. I'll update the worker in a few days.

@asvetlov asvetlov self-assigned this Aug 28, 2016

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Aug 28, 2016

Owner

@asvetlov Thanks! also as of now we don't have to only support wsgi. pure HTTP is ok too :) I will provide more details in the coming month.

Owner

benoitc commented Aug 28, 2016

@asvetlov Thanks! also as of now we don't have to only support wsgi. pure HTTP is ok too :) I will provide more details in the coming month.

@asvetlov

This comment has been minimized.

Show comment
Hide comment
@asvetlov

asvetlov Aug 28, 2016

Collaborator

@benoitc also I want to move aiohttp.wsgi into gunicorn (with keeping a copy in aiohttp for deprecation period).
There are no reasons to keep it in aiohttp itself -- guinicorn is the only user for the module.
Do you have an objection?

P.S.
WSGI support is limited, you know -- async reading of POST body is not allowed.

UPD.
See also aio-libs/aiohttp#1108

Collaborator

asvetlov commented Aug 28, 2016

@benoitc also I want to move aiohttp.wsgi into gunicorn (with keeping a copy in aiohttp for deprecation period).
There are no reasons to keep it in aiohttp itself -- guinicorn is the only user for the module.
Do you have an objection?

P.S.
WSGI support is limited, you know -- async reading of POST body is not allowed.

UPD.
See also aio-libs/aiohttp#1108

@asvetlov

This comment has been minimized.

Show comment
Hide comment
@asvetlov

asvetlov Aug 28, 2016

Collaborator

But deployment aiohttp applications behind gunicorn is quite popular -- people prefer to use it for forking several aiohttp workers with well-known rules.

Collaborator

asvetlov commented Aug 28, 2016

But deployment aiohttp applications behind gunicorn is quite popular -- people prefer to use it for forking several aiohttp workers with well-known rules.

@asvetlov

This comment has been minimized.

Show comment
Hide comment
@asvetlov

asvetlov Sep 12, 2016

Collaborator

Sorry, it takes more than few days but I didn't give up.
The issue is still in my todo list.
But I should publish new aiohttp release first.

Collaborator

asvetlov commented Sep 12, 2016

Sorry, it takes more than few days but I didn't give up.
The issue is still in my todo list.
But I should publish new aiohttp release first.

@tilgovi

This comment has been minimized.

Show comment
Hide comment
@tilgovi

tilgovi Oct 1, 2016

Collaborator

@asvetlov we've discussed packaging the workers separately and we have the gunicorn organization on GitHub already. Maybe aiohttp can be the first worker packaged and added to that organization.

Collaborator

tilgovi commented Oct 1, 2016

@asvetlov we've discussed packaging the workers separately and we have the gunicorn organization on GitHub already. Maybe aiohttp can be the first worker packaged and added to that organization.

@berkerpeksag

This comment has been minimized.

Show comment
Hide comment
@berkerpeksag

berkerpeksag Oct 2, 2016

Collaborator

Maybe aiohttp can be the first worker packaged and added to that organization.

+1 from me.

Collaborator

berkerpeksag commented Oct 2, 2016

Maybe aiohttp can be the first worker packaged and added to that organization.

+1 from me.

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Dec 27, 2016

Owner

still. I would appreciate to have the issue fixed. so we can next go further eventually. Any idea of the work to do for it @asvetlov ?

Owner

benoitc commented Dec 27, 2016

still. I would appreciate to have the issue fixed. so we can next go further eventually. Any idea of the work to do for it @asvetlov ?

@benoitc benoitc added this to Reported, waiting in Upstream bugs Feb 26, 2017

berkerpeksag added a commit to berkerpeksag/gunicorn that referenced this issue Aug 9, 2017

berkerpeksag added a commit to berkerpeksag/gunicorn that referenced this issue Aug 10, 2017

berkerpeksag added a commit to berkerpeksag/gunicorn that referenced this issue Aug 22, 2017

berkerpeksag added a commit that referenced this issue Oct 31, 2017

fofanov pushed a commit to fofanov/gunicorn that referenced this issue Mar 16, 2018

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