You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Yao Xu (Code Review)" <ge...@cloudera.org> on 2019/03/01 03:58:54 UTC

[kudu-CR] KUDU-2514 Support extra config for table.

Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12468 )

Change subject: KUDU-2514 Support extra config for table.
......................................................................


Patch Set 2:

(1 comment)

Hi, todd. Thanks for review.

http://gerrit.cloudera.org:8080/#/c/12468/2/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:

http://gerrit.cloudera.org:8080/#/c/12468/2/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@364
PS2, Line 364:   public AlterTableOptions alterHistoryMaxAgeSec(int historyMaxAgeSec) {
> I think it's worth a discussion whether we want to add a separate API for e
How about define API like this?
public AlterTableOptions alterTableExtraConfig(String key, String value);
public AlterTableOptions alterTableExtraConfig(String key, int value);
By the way, maybe TableExtraConfigPB also needs to be defined as key-value. What do you think?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0514507dca95602a97e954d1db464b907e073aae
Gerrit-Change-Number: 12468
Gerrit-PatchSet: 2
Gerrit-Owner: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Fri, 01 Mar 2019 03:58:54 +0000
Gerrit-HasComments: Yes