Skip to content

Log connect failures in BalancedClickhouseDataSource#501

Merged
zhicwu merged 1 commit into
ClickHouse:developfrom
chrisribble:master
Jan 15, 2021
Merged

Log connect failures in BalancedClickhouseDataSource#501
zhicwu merged 1 commit into
ClickHouse:developfrom
chrisribble:master

Conversation

@chrisribble
Copy link
Copy Markdown
Contributor

It is very difficult to debug connect failures while using BalancedClickhouseDataSource as the code swallows all connect exceptions.

This adds a log so that it's actually possible to figure out why connect is failing for one or more JDBC URLs. I wasn't totally sure what the ethos is in this project WRT how much to log, so I started at debug, but would be happy to increase the log level.

@chrisribble
Copy link
Copy Markdown
Contributor Author

@zhicwu @alexey-milovidov @qoega would it be possible to get some feedback on this change?

Please let me know if you would like me to approach this differently.

@zhicwu zhicwu added this to the 0.2.5 release milestone Jan 3, 2021
@zhicwu
Copy link
Copy Markdown
Contributor

zhicwu commented Jan 4, 2021

Sorry for the late response @chrisribble. It shouldn't take this long for adding log for debugging ;) I believe you've already made the change at your end, di you find anything interesting?

Anyway, could you change merge target of this PR from master to develop so that I can merge?

@chrisribble chrisribble changed the base branch from master to develop January 15, 2021 18:24
@chrisribble
Copy link
Copy Markdown
Contributor Author

Sorry for the late response @chrisribble. It shouldn't take this long for adding log for debugging ;) I believe you've already made the change at your end, di you find anything interesting?

Anyway, could you change merge target of this PR from master to develop so that I can merge?

@zhicwu

Yes, by adding the change, we were able to discover that we had an authentication problem (the exception stack trace included information indicating that the credentials were not correct).

I've updated the PR to target the develop branch

@zhicwu zhicwu merged commit f30549c into ClickHouse:develop Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants