----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15151/#review28677 -----------------------------------------------------------
itests/hive-unit/src/test/java/org/apache/hive/service/cli/CLIServiceTest.java <https://reviews.apache.org/r/15151/#comment55613> Does this patch make any changes to CLIServiceTest or ThriftCLIServiceTest, or does it just move these files from service/ to itests/ ? If it does make changes can we move the files in a different patch? service/if/TCLIService.thrift <https://reviews.apache.org/r/15151/#comment55611> Need to add HIVE_CLI_SERVICE_PROTOCOL_V5 and update any references to HIVE_CLI_SERVICE_PROTOCOL_V4. service/if/TCLIService.thrift <https://reviews.apache.org/r/15151/#comment55615> Please reuse TStatus instead of adding a new struct. service/if/TCLIService.thrift <https://reviews.apache.org/r/15151/#comment55610> This modification will break compatibility between upversion/downversion clients and servers since it modifies the type of an existing field. It's possible to avoid this problem by instead adding a new "TStatus operationStatus" field. service/src/java/org/apache/hive/service/cli/CLIService.java <https://reviews.apache.org/r/15151/#comment55619> Please push this logic into OperationManager.getOperationStatus() service/src/java/org/apache/hive/service/cli/operation/Operation.java <https://reviews.apache.org/r/15151/#comment55621> Would it make sense to replace getState() and getRunException() with a getStatus() method that returns an object wrapping the operationState and operationException? If not, please change the name of getRunException to getException(), and add a comment explaining under what conditions this method will return a non-null value. service/src/java/org/apache/hive/service/cli/operation/OperationManager.java <https://reviews.apache.org/r/15151/#comment55625> s/getOperationRunException/getOperationException/ service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15151/#comment55626> s/runException/operationException/ service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java <https://reviews.apache.org/r/15151/#comment55633> May as well just push this into Operation. service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceClient.java <https://reviews.apache.org/r/15151/#comment55631> Todo: set the status information. - Carl Steinbach On Nov. 11, 2013, 7:23 p.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15151/ > ----------------------------------------------------------- > > (Updated Nov. 11, 2013, 7:23 p.m.) > > > Review request for hive, Prasad Mujumdar and Thejas Nair. > > > Bugs: HIVE-5230 > https://issues.apache.org/jira/browse/HIVE-5230 > > > Repository: hive-git > > > Description > ------- > > [HIVE-4617|https://issues.apache.org/jira/browse/HIVE-4617] provides support > for async execution in HS2. When a background thread gets an error, currently > the client can only poll for the operation state and also the error with its > stacktrace is logged. However, it will be useful to provide a richer error > response like thrift API does with TStatus (which is constructed while > building a Thrift response object). > > > Diffs > ----- > > > itests/hive-unit/src/test/java/org/apache/hive/service/cli/CLIServiceTest.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceTest.java > PRE-CREATION > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java fce19bf > service/if/TCLIService.thrift 1f49445 > service/src/java/org/apache/hive/service/cli/CLIService.java 8c85386 > service/src/java/org/apache/hive/service/cli/CLIServiceClient.java 14ef54f > service/src/java/org/apache/hive/service/cli/EmbeddedCLIServiceClient.java > 9dca874 > service/src/java/org/apache/hive/service/cli/HiveSQLException.java 74e8b94 > service/src/java/org/apache/hive/service/cli/ICLIService.java f647ce6 > service/src/java/org/apache/hive/service/cli/OperationState.java 1ec6bd1 > service/src/java/org/apache/hive/service/cli/OperationStatus.java > PRE-CREATION > service/src/java/org/apache/hive/service/cli/operation/Operation.java > 6f4b8dc > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > bcdb67f > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > 4ee1b74 > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > 9df110e > > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceClient.java > 9bb2a0f > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java cd9d99a > > service/src/test/org/apache/hive/service/cli/thrift/ThriftCLIServiceTest.java > ff7166d > > Diff: https://reviews.apache.org/r/15151/diff/ > > > Testing > ------- > > > Thanks, > > Vaibhav Gumashta > >
