You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2020/09/01 13:36:50 UTC
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Hello Grant Henke,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/16394
to review the following change.
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
KUDU-3185 Add table owner to web UI and tools
Table ownership has been added earlier, but currently there's no easy
way to view the owner of a table. This commit adds this information to
the `kudu table describe` output and the web UI to the /table and
/tables pages.
Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
---
M src/kudu/master/master_path_handlers.cc
M src/kudu/tools/tool_action_table.cc
M www/table.mustache
M www/tables.mustache
4 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16394/1
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/5/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/5/src/kudu/tools/tool_action_table.cc@237
PS5, Line 237: cout << "OWNER " << table->owner() << endl;
Nit: Should we omit this line all together if the owner is unset/empty?
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 02 Sep 2020 17:17:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16394/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/16394/3/src/kudu/tools/kudu-tool-test.cc@5947
PS3, Line 5947: , static_cast<string>("")
Is it possible to remove this argument or that's the only way to create a table with no owner? As I can see, CreateKuduTable() has a parameter by default, and I'd think that's how you create a table with no owner.
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc@237
PS1, Line 237: cout << "OWNER " << table->owner() << endl;
> Sure, added a test case for describe. We don't have any testing for web UI
I don't recall any automation for the web UI testing.
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 18:28:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc@237
PS1, Line 237: cout << "OWNER " << table->owner() << endl;
> Mind adding one? It would be good to have coverage on the tool output.
Sure, added a test case for describe. We don't have any testing for web UI do we?
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 16:44:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16394/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/16394/2/src/kudu/tools/kudu-tool-test.cc@5834
PS2, Line 5834: ser
nit: typo ?
http://gerrit.cloudera.org:8080/#/c/16394/2/src/kudu/tools/kudu-tool-test.cc@5925
PS2, Line 5925: kOwner
Does it make sense to add a test scenario to verify the output of the tool when table doesn't have an owner?
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 17:12:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 4:
> > Patch Set 3: -Code-Review
> >
> > Whoops, it seems the newly added test fails:
> >
> > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/tools/kudu-admin-test.cc:1759
> > Value of: stdout
> > Expected: has substring "(\n key INT32 NOT NULL,\n int_val
> INT32 NOT NULL,\n string_val STRING NULLABLE,\n PRIMARY KEY
> (key)\n)\nRANGE (key) (\n PARTITION UNBOUNDED\n)\nREPLICAS 1"
> > Actual: "TABLE TestTable (\n key INT32 NOT NULL,\n
> int_val INT32 NOT NULL,\n string_val STRING NULLABLE,\n
> PRIMARY KEY (key)\n)\nRANGE (key) (\n PARTITION
> UNBOUNDED\n)\nOWNER slave\nREPLICAS 1\n" (of type std::string)
>
> Apparently we did have a test for describe table, just not in the
> file I was expecting it to be in. Updated the test and moved the
> no-owner case to kudu-admin-test as well.
Ah, indeed -- that's a test in other file, named the same as the newly introduced one in PS3.
BTW, this time it seems IWYU isn't too happy:
>>> Fixing #includes in '/home/jenkins-slave/workspace/kudu-master/1/src/kudu/integration-tests/ts_itest-base.cc'
@@ -22,7 +22,6 @@
#include <ostream>
#include <set>
#include <string>
-#include <type_traits>
#include <unordered_map>
#include <unordered_set>
#include <utility>
IWYU would have edited 1 fil
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 21:22:45 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/16394/3/src/kudu/tools/kudu-tool-test.cc@5947
PS3, Line 5947: , static_cast<string>("")
> If you don't set an owner, the owner will be the current user, so you need
Thank you for the clarification.
I guess the most important thing is to make sure the tool works as expected in case of older installation where no owner is recorded for a table, so we have explicit expectations about the output and the behavior of the tool. And this test scenario covers that, and this is great.
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 19:19:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 4:
> Patch Set 3: -Code-Review
>
> Whoops, it seems the newly added test fails:
>
> /home/jenkins-slave/workspace/kudu-master/1/src/kudu/tools/kudu-admin-test.cc:1759
> Value of: stdout
> Expected: has substring "(\n key INT32 NOT NULL,\n int_val INT32 NOT NULL,\n string_val STRING NULLABLE,\n PRIMARY KEY (key)\n)\nRANGE (key) (\n PARTITION UNBOUNDED\n)\nREPLICAS 1"
> Actual: "TABLE TestTable (\n key INT32 NOT NULL,\n int_val INT32 NOT NULL,\n string_val STRING NULLABLE,\n PRIMARY KEY (key)\n)\nRANGE (key) (\n PARTITION UNBOUNDED\n)\nOWNER slave\nREPLICAS 1\n" (of type std::string)
Apparently we did have a test for describe table, just not in the file I was expecting it to be in. Updated the test and moved the no-owner case to kudu-admin-test as well.
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 21:09:07 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 3: -Code-Review
Whoops, it seems the newly added test fails:
/home/jenkins-slave/workspace/kudu-master/1/src/kudu/tools/kudu-admin-test.cc:1759
Value of: stdout
Expected: has substring "(\n key INT32 NOT NULL,\n int_val INT32 NOT NULL,\n string_val STRING NULLABLE,\n PRIMARY KEY (key)\n)\nRANGE (key) (\n PARTITION UNBOUNDED\n)\nREPLICAS 1"
Actual: "TABLE TestTable (\n key INT32 NOT NULL,\n int_val INT32 NOT NULL,\n string_val STRING NULLABLE,\n PRIMARY KEY (key)\n)\nRANGE (key) (\n PARTITION UNBOUNDED\n)\nOWNER slave\nREPLICAS 1\n" (of type std::string)
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 19:20:08 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/5/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/5/src/kudu/tools/tool_action_table.cc@237
PS5, Line 237: cout << "OWNER " << table->owner() << endl;
> Hm not sure tbh. You can't create tables without an owner post-1.13, so onl
I am okay with leaving it. We can always change it if needed.
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 02 Sep 2020 17:34:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Greg Solovyev,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16394
to look at the new patch set (#4).
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
KUDU-3185 Add table owner to web UI and tools
Table ownership has been added earlier, but currently there's no easy
way to view the owner of a table. This commit adds this information to
the `kudu table describe` output and the web UI to the /table and
/tables pages.
Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
---
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_table.cc
M www/table.mustache
M www/tables.mustache
6 files changed, 93 insertions(+), 53 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16394/4
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc@237
PS1, Line 237: cout << "OWNER " << table->owner() << endl;
> I couldn't find any, I did verify manually though.
Mind adding one? It would be good to have coverage on the tool output.
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 15:02:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Greg Solovyev (Code Review)" <ge...@cloudera.org>.
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 16:51:01 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc@237
PS1, Line 237: cout << "OWNER " << table->owner() << endl;
Is there a test where validating this could be tacked onto?
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 14:43:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
KUDU-3185 Add table owner to web UI and tools
Table ownership has been added earlier, but currently there's no easy
way to view the owner of a table. This commit adds this information to
the `kudu table describe` output and the web UI to the /table and
/tables pages.
Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Reviewed-on: http://gerrit.cloudera.org:8080/16394
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_table.cc
M www/table.mustache
M www/tables.mustache
6 files changed, 93 insertions(+), 54 deletions(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/5/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/5/src/kudu/tools/tool_action_table.cc@237
PS5, Line 237: cout << "OWNER " << table->owner() << endl;
> Nit: Should we omit this line all together if the owner is unset/empty?
Hm not sure tbh. You can't create tables without an owner post-1.13, so only old tables can exist without owners. Maybe add something like "OWNER <none>" or something would make more sense to indicate that the table doesn't have an owner. Should I add it or should we leave it as an empty string?
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 02 Sep 2020 17:33:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:
http://gerrit.cloudera.org:8080/#/c/16394/1/src/kudu/tools/tool_action_table.cc@237
PS1, Line 237: cout << "OWNER " << table->owner() << endl;
> Is there a test where validating this could be tacked onto?
I couldn't find any, I did verify manually though.
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 15:00:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16394
to look at the new patch set (#2).
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
KUDU-3185 Add table owner to web UI and tools
Table ownership has been added earlier, but currently there's no easy
way to view the owner of a table. This commit adds this information to
the `kudu table describe` output and the web UI to the /table and
/tables pages.
Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
---
M src/kudu/master/master_path_handlers.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
M www/table.mustache
M www/tables.mustache
5 files changed, 32 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16394/2
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16394/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/16394/2/src/kudu/tools/kudu-tool-test.cc@5834
PS2, Line 5834: use
> nit: typo ?
Done
http://gerrit.cloudera.org:8080/#/c/16394/2/src/kudu/tools/kudu-tool-test.cc@5925
PS2, Line 5925: kOwner
> Does it make sense to add a test scenario to verify the output of the tool
Done
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 18:21:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Greg Solovyev,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16394
to look at the new patch set (#5).
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
KUDU-3185 Add table owner to web UI and tools
Table ownership has been added earlier, but currently there's no easy
way to view the owner of a table. This commit adds this information to
the `kudu table describe` output and the web UI to the /table and
/tables pages.
Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
---
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_table.cc
M www/table.mustache
M www/tables.mustache
6 files changed, 93 insertions(+), 54 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16394/5
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, Greg Solovyev,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16394
to look at the new patch set (#3).
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
KUDU-3185 Add table owner to web UI and tools
Table ownership has been added earlier, but currently there's no easy
way to view the owner of a table. This commit adds this information to
the `kudu table describe` output and the web UI to the /table and
/tables pages.
Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
---
M src/kudu/master/master_path_handlers.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
M www/table.mustache
M www/tables.mustache
5 files changed, 45 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16394/3
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3185 Add table owner to web UI and tools
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16394 )
Change subject: KUDU-3185 Add table owner to web UI and tools
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16394/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/16394/3/src/kudu/tools/kudu-tool-test.cc@5947
PS3, Line 5947: , static_cast<string>("")
> Is it possible to remove this argument or that's the only way to create a t
If you don't set an owner, the owner will be the current user, so you need to explicitly create it with an empty owner. You also can't normally create table with no owner, hence the allow_empty_owner=true above which is test-only to be able to test edge edge cases along upgrades such as this.
--
To view, visit http://gerrit.cloudera.org:8080/16394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0791847b58925ec9818943c62fe4a1bc00359749
Gerrit-Change-Number: 16394
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Greg Solovyev <gs...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 18:33:17 +0000
Gerrit-HasComments: Yes