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

Fix #198: Use a thread for Celluloid, shutdown EM. #224

Merged
merged 1 commit into from
Sep 8, 2018

Conversation

dblock
Copy link
Collaborator

@dblock dblock commented Sep 8, 2018

Celluloid change is probably not the best as we create a thread per client now, but since it's going to be deprecated we can roll with it. Joining the actor from the integration spec didn't work.

@dangerpr-bot
Copy link

1 Warning
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#224](https://github.com/slack-ruby/slack-ruby-client/pull/224): Fix #198: use a thread for celluloid, shutdown em - [@dblock](https://github.com/dblock).

Generated by 🚫 danger

@dblock dblock merged commit 1bfa20f into slack-ruby:master Sep 8, 2018
@dblock dblock deleted the fix-198 branch September 8, 2018 16:11
@ioquatix
Copy link
Contributor

Sounds like a reasonable change.

One thing to consider is that a celluloid actor runs on it’s own thread anyway. Celluloid has global state which is very hard to isolate. I commend you on your approach and I will take a more detailed look in about 2 weeks.

dblock added a commit to dblock/slack-ruby-client that referenced this pull request Sep 29, 2018
dblock added a commit to dblock/slack-ruby-client that referenced this pull request Sep 30, 2018
dblock added a commit that referenced this pull request Sep 30, 2018
Replace Thread by Actor, undoing #224.
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.

3 participants