> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 402
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line402>
> >
> >     Please use a switch statement here.

We can use a switch here but in java enum constant cannot be qualified in a 
case label. Which means basically within each case we'll have to use 
case(CANCELED) instead of case(OperationState.CANCELED) [<-- not allowed]. I 
guess the current approach is better.


> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java, 
> > line 220
> > <https://reviews.apache.org/r/15435/diff/3/?file=382338#file382338line220>
> >
> >     Should probably use a long and HiveConf.getLongVar instead of ints.

Done (although with int the max value could reach in several hours. Not sure if 
we are looking for a timeout of more than that). 


> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 356
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line356>
> >
> >     Please try to avoid using concrete collection types on the LHS or 
> > assignments or in method parameter lists, i.e. use Map<String, String> 
> > instead of HashMap<String, String>.

Done


> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 384
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line384>
> >
> >     Replace "hive.server2.long.polling.timeout" with 
> > HiveConf.ConfVars.HIVE_SERVER2_LONG_POLLING_TIMEOUT.varname.

Done


> On Nov. 12, 2013, 10:27 p.m., Carl Steinbach wrote:
> > service/src/test/org/apache/hive/service/cli/CLIServiceTest.java, line 406
> > <https://reviews.apache.org/r/15435/diff/3/?file=382339#file382339line406>
> >
> >     Formatting.

Done


- Vaibhav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15435/#review28749
-----------------------------------------------------------


On Nov. 12, 2013, 2:16 a.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15435/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2013, 2:16 a.m.)
> 
> 
> Review request for hive, Carl Steinbach and Thejas Nair.
> 
> 
> Bugs: HIVE-5217
>     https://issues.apache.org/jira/browse/HIVE-5217
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add long polling to asynchronous execution in HiveServer2
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 4f32390 
>   conf/hive-default.xml.template fe7141e 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 
> 4ee1b74 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java cd9d99a 
> 
> Diff: https://reviews.apache.org/r/15435/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Reply via email to