Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17607 )

Change subject: [metrics] KUDU-3269: Add UUID of the server into the metrics 
output
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:

http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1386
PS2, Line 1386: //Validate the hostname and the UUID of the server from the 
metrics webpage
nit: add a space before 'V'


http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/master/master-test.cc@1388
PS2, Line 1388:
nit: indent should be 2 spaces

If you are using vim, adding the following into .vimrc should help:

set expandtab
set shiftwidth=2                                                             
set tabstop=2


http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:

http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@852
PS2, Line 852:   if (GetHostname(&server_hostname).ok()) {
I guess it's worth trying to fetch FQDN?

    string hostname;
    if (!GetFQDN(&hostname).ok()) {
      WARN_NOT_OK(GetHostname(&hostname), "could not determine hostname");
    }


http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/server/server_base.cc@855
PS2, Line 855:       metric_entity_->SetAttribute("hostname", "Could not be 
determined" );
> I think I would prefer an empty string given the presence of a value could
+1


http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:

http://gerrit.cloudera.org:8080/#/c/17607/2/src/kudu/tserver/tablet_server-test.cc@3820
PS2, Line 3820:
nit: the indent should be 2 spaces



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d6d1e96f8067a7b1593da4f9d0e1931d3001016
Gerrit-Change-Number: 17607
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 23 Jun 2021 14:27:52 +0000
Gerrit-HasComments: Yes

Reply via email to