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

Replace Thread by Actor, undoing #224. #228

Merged
merged 1 commit into from
Sep 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
### 0.13.1 (Next)

* [#228](https://github.com/slack-ruby/slack-ruby-client/pull/228): Replace `Thread` with `Actor` in Celluloid async support - [@dblock](https://github.com/dblock).
* Your contribution here.

### 0.13.0 (2018/9/8)

* [#219](https://github.com/slack-ruby/slack-ruby-client/pull/219): Added support for `async-websocket` - [@dblock](https://github.com/dblock), [@ioquatix](https://github.com/ioquatix).
* [#224](https://github.com/slack-ruby/slack-ruby-client/pull/224): Celluloid async support now uses a thread per client - [@dblock](https://github.com/dblock).
* [#224](https://github.com/slack-ruby/slack-ruby-client/pull/224): Celluloid async support now uses a thread per client - [@dblock](https://github.com/dblock).
* [#224](https://github.com/slack-ruby/slack-ruby-client/pull/224): Attempt to shutdown EventMachine if a reactor wasn't already running on start - [@dblock](https://github.com/dblock).

### 0.12.0 (2018/8/20)
Expand Down
21 changes: 18 additions & 3 deletions lib/slack/real_time/concurrency/celluloid.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ def write(data)
end

def start_async(client)
Thread.new do
client.run_loop
end
@client = client
Actor.new(future.run_client_loop)
end

def run_client_loop
@client.run_loop
end

def connected?
Expand All @@ -79,6 +82,18 @@ def connected?

protected

class Actor
attr_reader :future

def initialize(future)
@future = future
end

def join
@future.value
end
end

def build_socket
socket = ::Celluloid::IO::TCPSocket.new(addr, port)
socket = ::Celluloid::IO::SSLSocket.new(socket, build_ssl_context) if secure?
Expand Down
9 changes: 4 additions & 5 deletions spec/integration/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

let(:client) { Slack::RealTime::Client.new(token: ENV['SLACK_API_TOKEN']) }

let(:connection) do
def start
# starts the client and pushes an item on a queue when connected
client.start_async do |driver|
driver.on :open do |data|
Expand All @@ -55,8 +55,8 @@ def start_server
logger.debug "#start_server, waiting #{dt} second(s)"
sleep dt # prevent Slack 429 rate limit errors
# start server and wait for on :open
c = connection
logger.debug "connection is #{c}"
@server = start
logger.debug "started #{@server}"
queue.pop_with_timeout(5)
end

Expand All @@ -74,7 +74,7 @@ def stop_server

after do
wait_for_server
connection.join
@server.join if @server.is_a?(::Thread)
end

context 'client connected' do
Expand Down Expand Up @@ -126,7 +126,6 @@ def stop_server

client.on :close do |data|
logger.debug "client.on :close, data=#{data}"
expect(client.started?).to be true
@close_called = true
end

Expand Down