Skip to content
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

Propagate client thumbstone when a client creation fails #221

Merged
merged 1 commit into from
Sep 27, 2019

Conversation

nicktorwald
Copy link

Propagate client thumbstone when a client creation fails

Closes: #30

@nicktorwald nicktorwald force-pushed the nicktorwald/gh-30-socket-provider-error branch from f4c2727 to 5235028 Compare September 10, 2019 11:02
@coveralls
Copy link

coveralls commented Sep 10, 2019

Coverage Status

Coverage increased (+0.05%) to 76.893% when pulling ed99a9b on nicktorwald/gh-30-socket-provider-error into 2b32e80 on master.

Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

I'm okay with the changes.

Maybe it worth to add a test with the default socket provider (don't sure either catch real 'connection refuse' error or mock get() method). It also seems that it worth to test the cluster client with its default socket provider (this is the case on which a customer asks us how to check what is going wrong). After that we'll sure that a user will see a real reason in case of some connectivity problems (during a first connection as well as during a reconnection later).

src/test/java/org/tarantool/ClientReconnectIT.java Outdated Show resolved Hide resolved
@nicktorwald nicktorwald force-pushed the nicktorwald/gh-30-socket-provider-error branch from 5235028 to 868b377 Compare September 11, 2019 11:47
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

I would find ConnectException where it is in causes just in case. Other then that I'm okay. LGTM.

@nicktorwald nicktorwald force-pushed the nicktorwald/gh-30-socket-provider-error branch 2 times, most recently from 6e93879 to ab1c2f6 Compare September 24, 2019 10:49
@Totktonada
Copy link
Member

I'm ok.

@nicktorwald nicktorwald force-pushed the nicktorwald/gh-30-socket-provider-error branch from ab1c2f6 to ed99a9b Compare September 24, 2019 12:48
@nicktorwald nicktorwald merged commit b4f1d6d into master Sep 27, 2019
@nicktorwald nicktorwald deleted the nicktorwald/gh-30-socket-provider-error branch September 27, 2019 09:54
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.

Propagate socketProvider.get() exception to a connection state
3 participants