Closed pseudomuto closed 10 years ago
This PR appears to make automatic retry default behaviour. While I personally don't mind, it may be a bad idea to change this. It may break retry functionality that people have built into their applications, and introduce unexpected behaviour when people update from a previous version.
Maybe this should be a client init option?
Hey @pseudomuto and @borc I agree that this is a good feature to have but we should allow people to enable it when instantiating a client rather than having it be the default behavior. If we can change that then I'll test and merge =)
Great point! Someone might not even know about it and that's definitely a problem. I'll find a way to make this optional
Thanks @pseudomuto !
The more I think about this the more I think maybe this should not be added here. It would add a gem dependency for something people might not even use.
Perhaps a gist along with a link in the README would be better. What do you guys think?
No, I do think it would be a useful part of the library, just that it should be optional. But if you'd rather not do that, then we can totally just go with the latter option.
Hey guys, I've updated the code to only retry timeouts if :retry_timeouts => true
is passed into the client initializer.
The way it works is by extending a specific instance of XMLRPC::Client
with the XMLRPCRetryable
module included here rather than monkey patching the class itself.
I've also updated the README file. What do you think?
That makes sense. Removed!
@zachfeldman would you be able verify CI is passing?
Sure @pseudomuto , sorry for the delay! I will definitely be taking a look at this this weekend.
Sweet!
@pseudomuto I'm having some issues when I try to run tests, will once again take a look this weekend. Sorry for the delay!
Cool...thanks man!
@pseudomuto test suite passes! Just make these changes
1) Remove Pry require line from spec_helper 2) Remove line 6 from users_spec (
3) Merge all commits into one final patch
Then we're good to go! Can't wait, sorry once again for the delay.
Hey @zachfeldman, I removed the pry line and squashed the commits. I didn't change user_spec though...did you want me to delete that line anyway?
Yeah @pseudomuto , I'd appreciate that!
And done!
@zachfeldman for review
Problem
Time outs can happen for all kinds of reasons. On occasion this happens with shared hosting sites that need to "wake up" your application before serving requests. When this happens, the solution is to simply make the call again.
Writing this retry code everywhere kinda sucks...
Solution
WrapXMLRPC::Client#call
in a retryable block. I've done this by using the equivalent toalias_method_chain
so that the retry is completely transparent to calling code (no changes required).Now all calls to
#connection.call
inRubypress::Client
will be retried if aTimeout::Error
orNet::ReadTimeout
occurs.Allow callers to pass
:retry_timeouts => true
when initializing a client. This will extend the specific instance ofXMLRPC::Client
being used by that client with retry functionality without affecting other clients.