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

Druid error handling #49

Merged
merged 2 commits into from
Jun 1, 2021
Merged

Druid error handling #49

merged 2 commits into from
Jun 1, 2021

Conversation

MarcPer
Copy link
Contributor

@MarcPer MarcPer commented May 30, 2021

The main two goals of this PR are:

  • don't retry requests to Druid when it indicates it's not going to work;
  • forward the error details provided by Druid to the library user.

To achieve the first goal, we have to override go-retryablehttp's CheckRetry function, adding cases when the request should not be retried. This depends not only on the response status code from Druid, but also on the response body, as indicated from the different error codes detailed in Druid's documentation.

To handle the second goal, both CheckRetry and ErrorHandler need to be overriden. CheckRetry needs to return the error received from Druid, and ErrorHandler needs to pass this error forward, as well as perform some clean-up.

@vigith vigith requested a review from jbguerraz May 31, 2021 17:44
Copy link
Contributor

@jbguerraz jbguerraz left a comment

Choose a reason for hiding this comment

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

LGTM, thank you :)

@jbguerraz jbguerraz merged commit c12bfaf into grafadruid:master Jun 1, 2021
@MarcPer MarcPer deleted the druidErrorHandling branch June 1, 2021 16:27
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.

2 participants