-
Notifications
You must be signed in to change notification settings - Fork 27
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
Differentiate different types of Drift failures #37
base: master
Are you sure you want to change the base?
Conversation
It's sometimes useful to log the reason for a failure. By separating out the specific failure reasons into different exceptions, clients can now inspect the underlying failure reason. This may be useful for logging, or for publication of custom metrics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just delete the unused class
} | ||
|
||
RetriesFailedException retriesFailedException = new RetriesFailedException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can delete the class for this now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RetriesFailedException has been renamed as DriftRetriesFailedException
int overloadedRejects, | ||
Set<? extends Address> attemptedAddresses) | ||
{ | ||
super(format("Max retry time (%s) exceeded", maxRetryTime), invocationAttempts, retryTime, failedConnections, overloadedRejects, attemptedAddresses); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These exception classes are structurally identical so maybe they could be one? These could be reason codes. Don't feel very strongly about it though.
Is there some other idiom you are following?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall. % comments from Ariel on using codes .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed the rename.
f6789f1
to
ffd0e7f
Compare
It's sometimes useful to log the reason for a failure. By separating out the specific
failure reasons into different exceptions, clients can now inspect the underlying failure
reason. This may be useful for logging, or for publication of custom metrics.