[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-02 Thread Attila Bukor (Code Review)
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 
---
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-02 Thread Grant Henke (Code Review)
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 02 Sep 2020 17:34:20 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-02 Thread Attila Bukor (Code Review)
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 " 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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 02 Sep 2020 17:33:22 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-02 Thread Grant Henke (Code Review)
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 02 Sep 2020 17:17:56 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Alexey Serbin (Code Review)
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 "(\nkey INT32 NOT NULL,\nint_val
 > INT32 NOT NULL,\nstring_val STRING NULLABLE,\nPRIMARY KEY
 > (key)\n)\nRANGE (key) (\nPARTITION UNBOUNDED\n)\nREPLICAS 1"
 > >   Actual: "TABLE TestTable (\nkey INT32 NOT NULL,\n
 > int_val INT32 NOT NULL,\nstring_val STRING NULLABLE,\n
 > PRIMARY KEY (key)\n)\nRANGE (key) (\nPARTITION
 > 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 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 21:22:45 +
Gerrit-HasComments: No


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 "(\nkey INT32 NOT NULL,\nint_val INT32 NOT 
> NULL,\nstring_val STRING NULLABLE,\nPRIMARY KEY (key)\n)\nRANGE (key) 
> (\nPARTITION UNBOUNDED\n)\nREPLICAS 1"
>   Actual: "TABLE TestTable (\nkey INT32 NOT NULL,\nint_val INT32 NOT 
> NULL,\nstring_val STRING NULLABLE,\nPRIMARY KEY (key)\n)\nRANGE (key) 
> (\nPARTITION 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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 21:09:07 +
Gerrit-HasComments: No


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Alexey Serbin (Code Review)
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 "(\nkey INT32 NOT NULL,\nint_val INT32 NOT 
NULL,\nstring_val STRING NULLABLE,\nPRIMARY KEY (key)\n)\nRANGE (key) 
(\nPARTITION UNBOUNDED\n)\nREPLICAS 1"
  Actual: "TABLE TestTable (\nkey INT32 NOT NULL,\nint_val INT32 NOT 
NULL,\nstring_val STRING NULLABLE,\nPRIMARY KEY (key)\n)\nRANGE (key) 
(\nPARTITION 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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 19:20:08 +
Gerrit-HasComments: No


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Alexey Serbin (Code Review)
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("")
> 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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 19:19:02 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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("")
> 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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 18:33:17 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Alexey Serbin (Code Review)
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("")
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 18:28:10 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 18:21:40 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Alexey Serbin (Code Review)
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 17:12:05 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Greg Solovyev (Code Review)
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 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Greg Solovyev 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 16:51:01 +
Gerrit-HasComments: No


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 16:44:18 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Grant Henke (Code Review)
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 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 15:02:11 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 15:00:46 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Grant Henke (Code Review)
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 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 01 Sep 2020 14:43:02 +
Gerrit-HasComments: Yes


[kudu-CR] KUDU-3185 Add table owner to web UI and tools

2020-09-01 Thread Attila Bukor (Code Review)
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 
Gerrit-Reviewer: Grant Henke