Wes McKinney has posted comments on this change. Change subject: Initial structure for the C++ hiveserver2 client. ......................................................................
Patch Set 12: (5 comments) a few comments from me just in case there is clarification needed http://gerrit.cloudera.org:8080/#/c/2645/12/src/hs2client/columnar-row-set.h File src/hs2client/columnar-row-set.h: Line 62: TypedColumn(const uint8_t* nulls, const std::vector<T>* data) > does this need to be public? it may help with unit testing http://gerrit.cloudera.org:8080/#/c/2645/12/src/hs2client/hs2service.h File src/hs2client/hs2service.h: Line 109: Status OpenSession(const std::string& user, const HS2ClientConfig& config, > is 'user' sufficient as an authentication mechanism? aside: we may want some SessionParams struct with things like user in it http://gerrit.cloudera.org:8080/#/c/2645/12/src/hs2client/hs2session.h File src/hs2client/hs2session.h: Line 48: std::shared_ptr<Operation>* operation) const; > the class comment for Operation says it's not thread-safe, so why return th The rationale is that the operation may be handed off to some other code. You could return unique_ptr, but then you would have to transfer it to a shared_ptr if you wished to share ownership. Operation might do well to become thread-safe at some point. Line 55: Status GetSchemas(std::shared_ptr<Operation>* operation) const; > what exactly does this do? (what is role of operation?) Operations provide FetchResults here, I think Line 59: Status GetTables(std::shared_ptr<Operation>* operation) const; > instead of returning the results of these metadata requests as generic oper Since the results are asynchronous, you're saying you'd want to wrap the async operation in some other struct type with a nicer API? (rather than needing do know that you need to inspect the result set to see the returned tables). -- To view, visit http://gerrit.cloudera.org:8080/2645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7be82d5897ad6093116915b17924674c4ae508a0 Gerrit-PatchSet: 12 Gerrit-Project: hs2client Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Wes McKinney <[email protected]> Gerrit-HasComments: Yes
