Henry Robinson has posted comments on this change.

Change subject: IMPALA-2767: Web UI call to force expire sessions
......................................................................


Patch Set 2: Code-Review+2

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3555/2/be/src/service/impala-http-handler.cc
File be/src/service/impala-http-handler.cc:

PS2, Line 67: static Status ParseSessionId(const Webserver::ArgumentMap& args, 
TUniqueId* id) {
            :   Webserver::ArgumentMap::const_iterator it = 
args.find("session_id");
            :   if (it == args.end()) {
            :     return Status("No 'session_id' argument found");
            :   } else {
            :     if (ParseId(it->second, id)) return Status::OK();
            :     return Status(Substitute("Could not parse 'session_id' 
argument: $0", it->second));
            :   }
            : }
how about refactoring this and ParseQueryId into a single method that takes an 
argument name?


PS2, Line 103: expire_session
close_session?


PS2, Line 104: ExpireSessionHandler
CloseSessionHandler?


Line 184:   Value message("Session closed successfully", 
document->GetAllocator());
nit: add the session ID here?


http://gerrit.cloudera.org:8080/#/c/3555/2/be/src/service/impala-http-handler.h
File be/src/service/impala-http-handler.h:

Line 95:   void ExpireSessionHandler(const Webserver::ArgumentMap& args,
Add a comment, like the method above.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2639993502a6deb5c24c3f1d055d82ade05ff67
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-HasComments: Yes

Reply via email to