Adar Dembo has posted comments on this change.

Change subject: RFC [java client] Redo how we manage exceptions
......................................................................


Patch Set 1:

(3 comments)

I'm generally a fan of inserting Status into our exceptions. It's more or less 
the same amount of information but it's far more structured, so it's actually 
usable by applications.

I'd also like to see us refine the "throws Exception" in nearly each method to 
"throws KuduException", though I wonder about how bad that will be for API 
compatibility.

http://gerrit.cloudera.org:8080/#/c/3055/1//COMMIT_MSG
Commit Message:

Line 15: This patch is the third of a series of attempts at simplifying this. 
With this
Would be useful to provide the other two attempts as context.


Line 16: patch we now only have two kinds of concrete exceptions. Here's the 
tree:
Hmm, but we still have PleaseThrottleException.


http://gerrit.cloudera.org:8080/#/c/3055/1/java/kudu-client/src/main/java/org/kududb/client/KuduException.java
File java/kudu-client/src/main/java/org/kududb/client/KuduException.java:

Line 37: public abstract class KuduException extends RuntimeException {
Out of curiosity, why does KuduException extend RuntimeException and not 
Exception? Don't we want it to be a checked exception?

The ship has probably sailed on this decision (i.e. we'd break API 
compatibility massively by changing it), but it'd be nice to know nonetheless.


-- 
To view, visit http://gerrit.cloudera.org:8080/3055
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba6e8a022d7a5391c3657cbdc9d3f06f951be048
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to