-
Notifications
You must be signed in to change notification settings - Fork 87
Conversation
… move forward.
This will change as of celluloid#121 and be updated in celluloid#123 after that.
…thout this being a problem.
Unix server goes into 0.6.0 ( but not for jRuby )
driver_env = DriverEnvironment.new(info, connection.socket) | ||
driver_env = DriverEnvironment.new(info, connection.socket) | ||
|
||
@socket = connection.hijack_socket | ||
@request_info = info | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just under this point, can we not use rack()
and use one of the other initialization methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see the comments in the original PR. Driver.server
calls Driver.rack
eventually, after parsing the headers on its own. websocket-driver does not depend on rack itself, but is built to expect rack apparently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Roger that. Can you verify this is operating as expected, with tests using all WebSocket
functionality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the angelo functional tests work with it, all the reel specs pass, and using it IRL with chrome websockets seems to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After you post a new PR, and we give @jcoglan a bit of time to possibly review your changes and comment, I will merge this and cut it as pre2
and we can see how it does in the wild then.
So far, extremely valiant and speedy effort @kenichi. Wondering if we have all the tests needed to verify this works in the wild. |
When you're ready, re-pull this to the |
@jcoglan any thoughts here? |
ah, oops, i'll rebase from that branch. |
see #189 |
this should resolve #186 and #187 - in my testing, this limited amount of rack mimicry works, no need for a full gem dependency and
Rack::MockRequest
instantiation.also, this adds a forward to the driver for
WebSocket#ping
.