Skip to content

RTS-1602 fix #346

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

Merged
merged 1 commit into from
Jan 18, 2017
Merged

RTS-1602 fix #346

merged 1 commit into from
Jan 18, 2017

Conversation

lukebakken
Copy link
Contributor

Add additional 500ms to client-side timeouts where appropriate. If the server-side timeout is the same value as the client-side one, tests that expect an error of <<"timeout">> may receive the client-side timeout atom instead (see this test)

Note: I'm going to review the timeout usage throughout the code to make sure that if a server-side timeout is in effect, the client-side one is 500ms longer.

@paegun - this branch is complete enough that it should resolve the verify_api_timeouts in your environment at this point.

@lukebakken lukebakken added this to the riak-erlang-client-2.5.2 milestone Jan 18, 2017
@lukebakken lukebakken self-assigned this Jan 18, 2017
@lukebakken lukebakken requested review from fadushin and paegun January 18, 2017 00:37
Copy link

@paegun paegun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

could DRY up the ServerTimeout (ST) and ClientTimeout (CT) bits, but IMO that is not required on these few duplications of code versus the perhaps awkward {ST, CT} = get_timeouts(Options).

@lukebakken
Copy link
Contributor Author

Yeah that'll be for the next PR that encompasses fixing all of the timeouts.

@lukebakken lukebakken changed the title WIP: RTS-1602 fix RTS-1602 fix Jan 18, 2017
@lukebakken lukebakken merged commit 09beedf into develop Jan 18, 2017
@lukebakken lukebakken deleted the fixes/lrb/rts-1602 branch January 18, 2017 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants