update plugin to use new shutdown stop semantics
#28
Conversation
| decorate(event) | ||
| queue << event | ||
| end | ||
| end # client.filter | ||
| rescue LogStash::ShutdownSignal | ||
| return | ||
| rescue Twitter::Error::TooManyRequests => e | ||
| @logger.warn("Twitter too many requests error, sleeping for #{e.rate_limit.reset_in}s") | ||
| sleep(e.rate_limit.reset_in) |
There was a problem hiding this comment.
It would be necessary to track this sleep, imagine the situation we're sleeping in, but aim to stop the input. This would be done easy by tracking the state and then using a thread.wakeup. You would also need to implement the stop method.
There was a problem hiding this comment.
isn't this true of all inputs that have the following "high-level" behavior
while !stop?
sleep 3
do_something()
end
There was a problem hiding this comment.
yes, and we updated them all to use now Stud.stopppable_stop from https://github.com/jordansissel/ruby-stud/blob/master/lib/stud/interval.rb#L76-L93 I recommend you update this PR to use it.
There was a problem hiding this comment.
updated to use stoppable sleep.
I tried doing
def stop
Stud.stop!
end
is that not the correct way to use Stud's stop mechanism?
instead I went with a block of { stop? }
|
Test pass for me, and kinda makes sense the way they are implemented 👍, just left you two small comments to figure it out and everything is fine!! good work here man! |
talevy
force-pushed
the
new_shutdown
branch
from
04ffd1d to
cde88a4
Compare
|
updated to reflect your comments, thanks! |
|
LGTM |
|
Merged sucessfully into master! |
Fixes #27.
depends on elastic/logstash#3895 and elastic/logstash-devutils#32