Alexey Serbin has posted comments on this change. Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error ......................................................................
Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/6640/4/src/kudu/integration-tests/security-unknown-tsk-itest.cc File src/kudu/integration-tests/security-unknown-tsk-itest.cc: Line 50: //DECLARE_int32(heartbeat_rpc_timeout_ms); > remove these? Done PS4, Line 83: torun > typo Done Line 128: ASSERT_NE(boost::none, authn_token); > I think this can be simplified to It seems there isn't 'bool operator()' for boost:optional. However, there is 'bool operator!()' and 'bool is_initialized()'. So, it seems the choice here is between ASSERT_FALSE(!authn_token); and ASSERT_TRUE(authn_token.is_initialized()); However, to me ASSERT_NE(boost::none, authn_token); looks better. I would like to keep this, if you don't mind. Line 152: ASSERT_OK(table_creator->table_name(kTableName) > When I initially read this, I got confused because I expected this to fail Thank you for pointing at that. Yes, exactly -- the connection to the master is cached and any further calls to the master would not try to validate the authn token if the connection kept open. Probably, I could set keepalive property for the connections to some small value and then make sure the call to the master returns ServiceUnavailable error. I'll do that and add corresponding comment: that way it will cover client-->master RPC path as well and it would be less confusing to readers, IMO. Line 174: auto cleanup = MakeScopedCleanup([&]() { > Might be simpler to use ElementDeleter from stl_util.h Done -- To view, visit http://gerrit.cloudera.org:8080/6640 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
