Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11681 )

Change subject: Add "service_name" as part of ConnectionId
......................................................................


Patch Set 2:

(3 comments)

Thanks for the review.

http://gerrit.cloudera.org:8080/#/c/11681/1/src/kudu/rpc/connection_id.h
File src/kudu/rpc/connection_id.h:

http://gerrit.cloudera.org:8080/#/c/11681/1/src/kudu/rpc/connection_id.h@56
PS1, Line 56:   // Returns a string representation of the object, not including 
the password field.
            :   std::string ToString() const;
> Looks like this isn't implemented. We can consider removing this.
Done


http://gerrit.cloudera.org:8080/#/c/11681/1/src/kudu/rpc/connection_id.cc
File src/kudu/rpc/connection_id.cc:

http://gerrit.cloudera.org:8080/#/c/11681/1/src/kudu/rpc/connection_id.cc@52
PS1, Line 52: string ConnectionId::ToString() const {
> Need to incorporate service_name_ here too.
Done


http://gerrit.cloudera.org:8080/#/c/11681/1/src/kudu/rpc/connection_id.cc@69
PS1, Line 69:   boost::hash_combine(seed, hostname_);
> We need to include the service_name_ as part of the HashCode too.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6767e631fd9530ea54f5ed63ff4c8c179ab216b2
Gerrit-Change-Number: 11681
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Tue, 16 Oct 2018 00:34:20 +0000
Gerrit-HasComments: Yes

Reply via email to