search

launchdarkly / ruby-eventsource

Server-Sent Events client for Ruby
Other
40 stars 16 forks source link

Expose whether the client is alive #13

Closed qcn closed 3 years ago

qcn commented 3 years ago

We used this gem for monitoring and found that it was useful to expose the aliveness status of the client to check if we needed to restart it.

eli-darkly commented 3 years ago

I don't quite understand the rationale for this, because the attribute you're checking, @stopped, only gets set to true if you have explicitly called close to shut down the client permanently. So 1. there's no way the client could have stopped itself without you already knowing about it, and 2. there is no restarting from that state.

qcn commented 3 years ago

The use-case is that we check this in a loop - if there's an error, we call close to shut down the client, but that's a response to an event. In the loop, if the client has been shut down, we spin up a new one. We could do this by setting a flag in our error handler and looping on that instead, but by exposing the stopped state of the client we link that behaviour explicitly to the client rather than an external flag.

eli-darkly commented 3 years ago

@qcn I see. I have no problem with adding this method, but I think it's a bit confusing to call it is_alive? when "alive" isn't a word that's used anywhere else in the API. Could you change it to is_closed? (and have it return the value of @stopped rather than the opposite) to clarify that it's referring to the state caused by close?

eli-darkly commented 3 years ago

Actually how about just closed? - Ruby's naming conventions don't require "is" for boolean getters.

qcn commented 3 years ago

@eli-darkly Great! I've made the change requested, thank you for the discussion! :)

eli-darkly commented 3 years ago

Version 2.1.0 has been released with this change.

qcn commented 3 years ago

@eli-darkly Thank you so much!