这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@drcrallen
Copy link
Contributor

Also had to update guava. Minimum 15.0 is needed to capture ByteSource.empty(). If this is undesirable it can be worked around.

@drcrallen drcrallen force-pushed the sequenceInputStreamResponseHandler branch 2 times, most recently from 1b9ef5a to fb91ba4 Compare April 28, 2015 23:36
Copy link
Contributor

Choose a reason for hiding this comment

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

thought some logs here will be nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had considered that overall, but most of the logs we have in the Druid impl have a lot of stuff solely related to that particular instance. As such, I would expect an implementation to override any method it needs logs for and log ass appropriate before calling the super method.

Since this is an internal handling, I'll add a simple log here in case higher methods do not catch the thrown interrupt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a bunch of debug logging

@drcrallen drcrallen force-pushed the sequenceInputStreamResponseHandler branch from fb91ba4 to 4bd3f54 Compare April 29, 2015 16:53
@nishantmonu51
Copy link
Contributor

+1

@xvrl
Copy link
Contributor

xvrl commented Sep 1, 2015

assuming this is the same code as in Druid +/- some logging seems fine to me

@drcrallen
Copy link
Contributor Author

@xvrl Yes

xvrl added a commit that referenced this pull request Sep 1, 2015
Port of the DirectDruidClient response handler.
@xvrl xvrl merged commit 3c5dd29 into master Sep 1, 2015
@xvrl xvrl deleted the sequenceInputStreamResponseHandler branch September 1, 2015 22:15
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.

5 participants