Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1807 (part 2): ban GetTableSchema for table createdness in 
clients
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8026/3/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File 
java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:

Line 361:    * considered to be successful.
> If you don't wait, you may operate on the table before the alter is finishe
That's why I don't like this. We don't want to document this because it's 
"implementation details" but then make it a public API.


http://gerrit.cloudera.org:8080/#/c/8026/3/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
File 
java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java:

Line 191:    * Whether to wait for all tablets to be created before the table 
creation is
> It's hard to doc this completely without mentioning some implementation det
I think we can't have our cake and eat it too, if we want to expose a public 
API that's about an implementation detail then we either document what it's 
doing or we don't add an obscure API. I do think a high level sketch of what 
this method does is required. Else what is it good for?


PS3, Line 199: setWait
> I agree with Dan; I think being more vague is better in this case. If we're
Will the new sub-operations also require new setters? We're basically already 
going down that route. I think there's room for more documentation.


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

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

Reply via email to