Dan Burkert has posted comments on this change.

Change subject: KUDU-1988: add support for advertised host:port info.
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6827/8/src/kudu/server/rpc_server.cc
File src/kudu/server/rpc_server.cc:

Line 117:   if (!options_.rpc_advertised_addresses.empty()) {
This should guard against port 0, as on line 111 above.


Line 210:   for (const Sockaddr& addr : rpc_advertised_addresses_) {
Same comment as in the webserver; this can just use the copy ctor.


http://gerrit.cloudera.org:8080/#/c/6827/8/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

Line 207:     
RETURN_NOT_OK(ParseAddressList(opts_.webserver_advertised_addresses,
This should check for 0 port as well.


Line 308:   for (const Sockaddr& addr : webserver_advertised_addresses_) {
This can be simpler as a copy construct:

    *addresses = webserver_advertised_addresses_;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6735ca5630fc4c426bf72d0b21d6ef452173a890
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Patrik Sundberg <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Patrik Sundberg <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

Reply via email to