[kudu-CR] Add a ToString() method to Proxy
Todd Lipcon has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Todd Lipcon has submitted this change and it was merged. Change subject: Add a ToString() method to Proxy .. Add a ToString() method to Proxy ReplicatedRpc takes the server proxy type as a template argument and uses its ToString() method to print out details in case of error. Usually this is RemoteTablet, which has a ToString() method, but that might not always be the case. In fact, in a test in a follow up patch ReplicatedRpc takes Proxy as the server proxy type and compilation would fail due to a missing ToString(). We could make ReplicatedRpc not use the ToString() method, but it seems very helpful to have it so this patch adds it to Proxy instead. Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Reviewed-on: http://gerrit.cloudera.org:8080/3502 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/rpc/proxy.cc M src/kudu/rpc/proxy.h M src/kudu/rpc/rpc-test.cc 3 files changed, 12 insertions(+), 0 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 9: Build Started http://104.196.14.100/job/kudu-gerrit/2339/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/2279/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/2259/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
David Ribeiro Alves has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3502/1/src/kudu/rpc/proxy.cc File src/kudu/rpc/proxy.cc: Line 110: return StringPrintf("%s@%s", service_name_.c_str(), conn_id_.ToString().c_str()); > Why do you need the c_str() calls? If it's a StringPrintf thing, maybe you Done -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 6: Build Started http://104.196.14.100/job/kudu-gerrit/2249/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2242/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3502 to look at the new patch set (#4). Change subject: Add a ToString() method to Proxy .. Add a ToString() method to Proxy ReplicatedRpc takes the server proxy type as a template argument and uses its ToString() method to print out details in case of error. Usually this is RemoteTablet, which has a ToString() method, but that might not always be the case. In fact, in a test in a follow up patch ReplicatedRpc takes Proxy as the server proxy type and compilation would fail due to a missing ToString(). We could make ReplicatedRpc not use the ToString() method, but it seems very helpful to have it so this patch adds it to Proxy instead. Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e --- M src/kudu/rpc/proxy.cc M src/kudu/rpc/proxy.h M src/kudu/rpc/rpc-test.cc 3 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/3502/4 -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2237/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Todd Lipcon has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 3: see note above about adding an assertion -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2165/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/2137/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Todd Lipcon has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 1: would be nice to add a new assertion to one of the RPC layer tests which calls this -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
Adar Dembo has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3502/1/src/kudu/rpc/proxy.cc File src/kudu/rpc/proxy.cc: Line 110: return StringPrintf("%s@%s", service_name_.c_str(), conn_id_.ToString().c_str()); Why do you need the c_str() calls? If it's a StringPrintf thing, maybe you can use strings::Substitute instead? -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add a ToString() method to Proxy
Kudu Jenkins has posted comments on this change. Change subject: Add a ToString() method to Proxy .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2007/ -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
David Ribeiro Alves has uploaded a new change for review. http://gerrit.cloudera.org:8080/3502 Change subject: Add a ToString() method to Proxy .. Add a ToString() method to Proxy ReplicatedRpc takes the server proxy type as a template argument and uses its ToString() method to print out details in case of error. Usually this is RemoteTablet, which has a ToString() method, but that might not always be the case. In fact, in a test in a follow up patch ReplicatedRpc takes Proxy as the server proxy type and compilation would fail due to a missing ToString(). We could make ReplicatedRpc not use the ToString() method, but it seems very helpful to have it so this patch adds it to Proxy instead. Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e --- M src/kudu/rpc/proxy.cc M src/kudu/rpc/proxy.h 2 files changed, 7 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/3502/1 -- To view, visit http://gerrit.cloudera.org:8080/3502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves