Fix default retry_limit #19
Conversation
Signed-off-by: Amol Shinde <[email protected]>
| retry_limit: @connection_retries.to_i, | ||
| retry_delay: @connection_retry_sleep.to_i, | ||
| }.merge(retry_options) | ||
| }) |
There was a problem hiding this comment.
This doesn't seem right to me. The usual order of preference overrides is: global -> ENV -> cmdline -> object -> method call... So that big loses to little. This change means that the options passed to the local session call are overridden by the object's ivars.
Also, a change like this really should have a test confirming the problem ad the fix.
|
I agree that this would benefit from a test to replicate and document the problem and the fix, otherwise it requires some slightly tricky replication to confirm from our end. I also see the issue with overriding (big losing to little), but with a confirming test and possibly a documenting comment explaining the override this could be closer to where it needs to be. 😄 |
|
Closing, review comments haven't been addressed in over 9 months. |
Signed-off-by: Amol Shinde [email protected]
Description
retry_limit was not being overridden by connection_retries. So, the program was trying(mostly 200 times) to make a connection for longer period of time.
Related Issue
Fixes #16
Types of changes
Checklist: