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 Celluloid support next to EventMachine #5

Closed
dblock opened this issue Aug 5, 2015 · 10 comments
Closed

Add Celluloid support next to EventMachine #5

dblock opened this issue Aug 5, 2015 · 10 comments

Comments

@dblock
Copy link
Collaborator

dblock commented Aug 5, 2015

See https://github.com/abstractive/slack-ruby-gem/commit/41a45ba11a11c087b501654a159e72f27526dd9d.

@digitalextremist
Copy link

I'll add Celluloid support, as I have in the other gem. Two things though.

  • I'd actually use Reel::IO ( celluloid/reel-io#2 ).
  • I'd also use this http gem instead.

@dblock
Copy link
Collaborator Author

dblock commented Aug 6, 2015

@digitalextremist are you saying we should replace Faraday for httprb? I would be open to it, but I am not sure it matters for slack-ruby-client given that HTTP communication is hardly the overhead.

@digitalextremist
Copy link

Correct, it is unrelated to the WebSocket aspect, so I will post a second ticket for this piece. Why I mention it is that this http gem is compatible with Celluloid::IO which makes it evented, and in my uses, I do have many concurrent requests taking place at once in many cases.

@mikz
Copy link
Contributor

mikz commented Sep 9, 2015

I'm really interested in this. Is there a way I could help?

@dblock
Copy link
Collaborator Author

dblock commented Sep 9, 2015

Please contribute @mikz!

@mikz
Copy link
Contributor

mikz commented Sep 9, 2015

I'd like not to clash on some ongoing work. I plan to make a prototype using just celluloid-io and websocket-driver without using reel-io.

@mikz
Copy link
Contributor

mikz commented Sep 15, 2015

@dblock @digitalextremist I did a prototype. See the code in dblock@ced303d. It successfully connected to slack and was answering with celluloid.

The celluloid integration is probably not very correct. It crashes on Ctrl-C.

What I'd like to do next:

  • cover it with tests that work for both concurrency options
  • refactor the socket / client so it is more understandable

@dblock
Copy link
Collaborator Author

dblock commented Sep 15, 2015

This is very good. I actually think this whole code should be a gem that abstracts various ways of doing concurrency.

@mikz
Copy link
Contributor

mikz commented Sep 15, 2015

@dblock opened #15 for discussion

tuned a bit the original prototype so now it detects what you have available
tried both realtime examples and they work

it is possible that some cases are broken, like closing connection or processing some messages
but rather to show it early than go the wrong path

@dblock
Copy link
Collaborator Author

dblock commented Nov 13, 2015

Closing via https://github.com/dblock/slack-ruby-client/pull/26. Please try Celluloid support on HEAD.

@dblock dblock closed this as completed Nov 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants