[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. [client] fix recently introduced ABI compatiblity issue This patch fixes the recently introduced ABI compatibility issue in Kudu C++ client once the KuduClient::ListTables() method was overloaded (see [1] for details). As I could see, the overloaded method was only used in the Kudu CLI tooling with no apparent justification why it's needed in the public client API. With that, I moved the method to be a part of the KuduClient::Data internal class. In addition, I renamed --list_statistics into --show_table_info for the `kudu table list` CLI tool and switched to the lower_camel_case for the tags used to output a table's stats. This is a follow-up to 3ccc434ea6dcbef15a12bd1734c8e66b69a611c5. [1] https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Reviewed-on: http://gerrit.cloudera.org:8080/17972 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 6 files changed, 88 insertions(+), 104 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao Gerrit-Comment-Date: Tue, 26 Oct 2021 22:14:43 + Gerrit-HasComments: No
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/17972/2/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/17972/2/src/kudu/client/client-internal.cc@438 PS2, Line 438: vector* tables_stats, > Yeah, I guess if we wanted an exported version of this, we could add to Kud Done -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao Gerrit-Comment-Date: Tue, 26 Oct 2021 20:45:13 + Gerrit-HasComments: Yes
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Hello Kudu Jenkins, Andrew Wong, yejiabao, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17972 to look at the new patch set (#3). Change subject: [client] fix recently introduced ABI compatiblity issue .. [client] fix recently introduced ABI compatiblity issue This patch fixes the recently introduced ABI compatibility issue in Kudu C++ client once the KuduClient::ListTables() method was overloaded (see [1] for details). As I could see, the overloaded method was only used in the Kudu CLI tooling with no apparent justification why it's needed in the public client API. With that, I moved the method to be a part of the KuduClient::Data internal class. In addition, I renamed --list_statistics into --show_table_info for the `kudu table list` CLI tool and switched to the lower_camel_case for the tags used to output a table's stats. This is a follow-up to 3ccc434ea6dcbef15a12bd1734c8e66b69a611c5. [1] https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 6 files changed, 88 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/17972/3 -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. Patch Set 2: Code-Review+2 (1 comment) Slightly lean towards renaming "stat" to "info", given stats typically summarize a lot of data, and the info we're showing is more than just a summarization of large data volumes. Don't feel strongly about it though. http://gerrit.cloudera.org:8080/#/c/17972/2/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/17972/2/src/kudu/client/client-internal.cc@438 PS2, Line 438: vector* tables_stats, Yeah, I guess if we wanted an exported version of this, we could add to KuduTableStatistics. Maybe we should in the future, or consider renaming this to TableInfo as it was, to avoid conflating the two (I don't feel strongly about it though). -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao Gerrit-Comment-Date: Tue, 26 Oct 2021 20:19:25 + Gerrit-HasComments: Yes
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/17972/1/src/kudu/client/client.cc File src/kudu/client/client.cc: http://gerrit.cloudera.org:8080/#/c/17972/1/src/kudu/client/client.cc@123 PS1, Line 123: > Would it be better to remove this statement because nobody used (ListTables Good point! Done. -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao Gerrit-Comment-Date: Tue, 26 Oct 2021 17:50:49 + Gerrit-HasComments: Yes
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Hello Kudu Jenkins, yejiabao, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/17972 to look at the new patch set (#2). Change subject: [client] fix recently introduced ABI compatiblity issue .. [client] fix recently introduced ABI compatiblity issue This patch fixes the recently introduced ABI compatibility issue in Kudu C++ client once the KuduClient::ListTables() method was overloaded (see [1] for details). As I could see, the overloaded method was only used in the Kudu CLI tooling with no apparent justification why it's needed in the public client API. With that, I moved the method to be a part of the KuduClient::Data internal class. In addition, I renamed --list_statistics into --show_stats for the `kudu table list` CLI tool and switched to the lower_camel_case for the tags used to output a table's stats. This is a follow-up to 3ccc434ea6dcbef15a12bd1734c8e66b69a611c5. [1] https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 6 files changed, 87 insertions(+), 103 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/17972/2 -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao Gerrit-Comment-Date: Tue, 26 Oct 2021 13:18:19 + Gerrit-HasComments: No
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
yejiabao has posted comments on this change. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17972/1/src/kudu/client/client.cc File src/kudu/client/client.cc: http://gerrit.cloudera.org:8080/#/c/17972/1/src/kudu/client/client.cc@123 PS1, Line 123: ; Would it be better to remove this statement because nobody used (ListTablesRequestPB and ListTablesResponsePB) -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao Gerrit-Comment-Date: Tue, 26 Oct 2021 06:43:49 + Gerrit-HasComments: Yes
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17972 ) Change subject: [client] fix recently introduced ABI compatiblity issue .. Patch Set 1: Verified+1 unrelated test failures: * TabletServerDiskError/TabletServerDiskErrorITest.TestSpaceAvailableMetrics/0 * ConcurrentRebalancersTest.TwoConcurrentRebalancers/1 -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao Gerrit-Comment-Date: Tue, 26 Oct 2021 04:49:09 + Gerrit-HasComments: No
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Alexey Serbin has removed a vote on this change. Change subject: [client] fix recently introduced ABI compatiblity issue .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: yejiabao
[kudu-CR] [client] fix recently introduced ABI compatiblity issue
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17972 Change subject: [client] fix recently introduced ABI compatiblity issue .. [client] fix recently introduced ABI compatiblity issue This patch fixes the recently introduced ABI compatibility issue in Kudu C++ client once the KuduClient::ListTables() method was overloaded (see [1] for details). As I could see, the overloaded method was only used in the Kudu CLI tooling with no apparent justification why it's needed in the public client API. With that, I moved the method to be a part of the KuduClient::Data internal class. In addition, I renamed --list_statistics into --show_stats for the `kudu table list` CLI tool and switched to the lower_camel_case for the tags used to output a table's stats. This is a follow-up to 3ccc434ea6dcbef15a12bd1734c8e66b69a611c5. [1] https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 6 files changed, 86 insertions(+), 95 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/17972/1 -- To view, visit http://gerrit.cloudera.org:8080/17972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I214d424eb0e91b90db813fde0e0b7150253bca91 Gerrit-Change-Number: 17972 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
