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

Add transports class variable to gunicorn workers to allow setting transports #123

Merged
merged 4 commits into from Dec 4, 2013

Conversation

berndtj
Copy link

@berndtj berndtj commented Jan 16, 2013

Nginx is often put in front of other webservers, but does not support web-sockets. This is generally fine, however if using the gunicorn worker, you cannot unset the websocket transport. This causes a traceback about a missing wsgi.web_socket key. There is already a closed issue about this.

Additionally, I added an Nginx specific worker class which sets the transport to xhr-polling. This appears to be the only transport which is currently working. It appears that there are several broken transports.

@moodh
Copy link

moodh commented Jan 16, 2013

nginx will support websockets soon (tm).

From twitter:
nginx web server ‏@nginxorg
@alexandernst Yes, WebSocket proxying should be there somewhere around early Feb'13. It's a contract work (as in "sponsored F/OSS dev")

@alexandernst
Copy link

Yeah! Really soon!! Great news! :D

@abourget
Copy link
Owner

I'd like to be able to set all the parameters that we find in server.py/SocketIOServer.init ... so having a special case for just transport isn't enough. Could you think of a way we could pass several config options down to the SocketIOServer ?

@berndtj
Copy link
Author

berndtj commented Feb 20, 2013

Well, I'm not sure gunicorn allows arbitrary values to be passed in via config. This sort of stops us from just using the gunicorn config with a generic worker class. However, I think just having the class variables available (like transport) so that a simple subclass can essentially configure is a reasonable way to solve this issue. I don't mind creating a custom subclass for my usage, but if the variables aren't exposed in the base class, then I'm stuck.

@sontek
Copy link
Collaborator

sontek commented Mar 31, 2013

@berndtj Could you clean this up to so that it can automatically be merged? Also, did you have any luck in making things more configurable? I started down this path once but got busy but I have time to look at it now if you do not.

Updates boostrap.py to resolve error when starting example project from scratch
@sontek
Copy link
Collaborator

sontek commented Apr 13, 2013

@berndtj Were you going to make this cleanly merge?

@berndtj
Copy link
Author

berndtj commented Apr 13, 2013

I can probably work on this tomorrow

Sent from my iPad

On Apr 13, 2013, at 4:02 PM, John Anderson notifications@github.com wrote:

@berndtj Were you going to make this cleanly merge?


Reply to this email directly or view it on GitHub.

@sontek
Copy link
Collaborator

sontek commented Apr 13, 2013

@berndtj Awesome, I'm going to push a new release on Monday and trying to get as much fixed up before then.

@berndtj
Copy link
Author

berndtj commented Apr 14, 2013

ok, should be clean to merge

@abourget
Copy link
Owner

Okay, so if that's the way to go to configure gunicorn, then we might as well document it this way, and invite people to create their own descendent with certains variables set.

In that case, I'd like to add all the variables that are now configurable (heartbeat_time, etc...).. and we could document that in the gunicorn section. With all the options, and pointing to the SocketIOServer docs, (which list those options).

@abourget
Copy link
Owner

Please read invitation to Wednesday September 18th's sprint: https://groups.google.com/forum/#!topic/gevent-socketio/2OIRKA8M2uE

sontek added a commit that referenced this pull request Dec 4, 2013
Add transports class variable to gunicorn workers to allow setting transports
@sontek sontek merged commit cee6d07 into abourget:master Dec 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants