You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vaibhav Gumashta <vg...@hortonworks.com> on 2017/07/07 20:54:11 UTC

Re: Review Request 58403: HIVE-16355 Service: embedded mode should only be available if service is loaded onto the classpath

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




jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
Lines 23 (patched)
<https://reviews.apache.org/r/58403/#comment254878>

    Is HiveConf needed?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
Lines 182 (patched)
<https://reviews.apache.org/r/58403/#comment254879>

    Would it be better to cast to ThriftCLIService (because that is what the embedded client essentially is)


- Vaibhav Gumashta


On April 19, 2017, 6:48 a.m., Zoltan Haindrich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58403/
> -----------------------------------------------------------
> 
> (Updated April 19, 2017, 6:48 a.m.)
> 
> 
> Review request for hive, Thejas Nair and Vaibhav Gumashta.
> 
> 
> Bugs: HIVE-16355
>     https://issues.apache.org/jira/browse/HIVE-16355
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> this is a small step toward separating some of the client code from the service module
> 
> * embedded mode is only supported if the hive-service module is loaded
> * use interface instead of implementation
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java fb18adb69232de9e595611cb95c025378d04f9fb 
>   service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java dcb6338d3977665b3c3e954acc5cd8dce4ac5048 
>   service/src/java/org/apache/hive/service/auth/KerberosSaslHelper.java ea2c689a4fb2e2ba6867b276044cbca0ac6899d3 
>   service/src/java/org/apache/hive/service/auth/PlainSaslHelper.java a161e1541e99fe30d7265a97bf196680d0fe905a 
> 
> 
> Diff: https://reviews.apache.org/r/58403/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>


Re: Review Request 58403: HIVE-16355 Service: embedded mode should only be available if service is loaded onto the classpath

Posted by Vaibhav Gumashta <vg...@hortonworks.com>.

> On July 7, 2017, 8:54 p.m., Vaibhav Gumashta wrote:
> >

Also, can you comment on the testing of this?


- Vaibhav


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


On April 19, 2017, 6:48 a.m., Zoltan Haindrich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58403/
> -----------------------------------------------------------
> 
> (Updated April 19, 2017, 6:48 a.m.)
> 
> 
> Review request for hive, Thejas Nair and Vaibhav Gumashta.
> 
> 
> Bugs: HIVE-16355
>     https://issues.apache.org/jira/browse/HIVE-16355
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> this is a small step toward separating some of the client code from the service module
> 
> * embedded mode is only supported if the hive-service module is loaded
> * use interface instead of implementation
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java fb18adb69232de9e595611cb95c025378d04f9fb 
>   service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java dcb6338d3977665b3c3e954acc5cd8dce4ac5048 
>   service/src/java/org/apache/hive/service/auth/KerberosSaslHelper.java ea2c689a4fb2e2ba6867b276044cbca0ac6899d3 
>   service/src/java/org/apache/hive/service/auth/PlainSaslHelper.java a161e1541e99fe30d7265a97bf196680d0fe905a 
> 
> 
> Diff: https://reviews.apache.org/r/58403/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>


Re: Review Request 58403: HIVE-16355 Service: embedded mode should only be available if service is loaded onto the classpath

Posted by Zoltan Haindrich <ki...@rxd.hu>.

> On July 7, 2017, 8:54 p.m., Vaibhav Gumashta wrote:
> >
> 
> Vaibhav Gumashta wrote:
>     Also, can you comment on the testing of this?

since currently I've not modified anything - the regular tests see the same as before...it would be great to separate the hs2's classpath from the client classpath - but that needs a more sophisticated setup; I've checked it with a simple jdbc client which connected to a minihs2.


> On July 7, 2017, 8:54 p.m., Vaibhav Gumashta wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
> > Lines 23 (patched)
> > <https://reviews.apache.org/r/58403/diff/2/?file=1694132#file1694132line23>
> >
> >     Is HiveConf needed?

no...I guess it was left here by an earlier version of the patch


> On July 7, 2017, 8:54 p.m., Vaibhav Gumashta wrote:
> > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
> > Lines 182 (patched)
> > <https://reviews.apache.org/r/58403/diff/2/?file=1694132#file1694132line183>
> >
> >     Would it be better to cast to ThriftCLIService (because that is what the embedded client essentially is)

ThriftCLISevice refers to at least SessionState and CLIService which are in turn pull in the Hive class; that's where all hell break loose :)

I see two possibilities:

* try to make ThriftCLIService free from classes like that...this might need more cleanup at the metastore level also...
* extract an interface for ThriftCLIService


- Zoltan


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


On April 19, 2017, 6:48 a.m., Zoltan Haindrich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58403/
> -----------------------------------------------------------
> 
> (Updated April 19, 2017, 6:48 a.m.)
> 
> 
> Review request for hive, Thejas Nair and Vaibhav Gumashta.
> 
> 
> Bugs: HIVE-16355
>     https://issues.apache.org/jira/browse/HIVE-16355
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> this is a small step toward separating some of the client code from the service module
> 
> * embedded mode is only supported if the hive-service module is loaded
> * use interface instead of implementation
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java fb18adb69232de9e595611cb95c025378d04f9fb 
>   service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java dcb6338d3977665b3c3e954acc5cd8dce4ac5048 
>   service/src/java/org/apache/hive/service/auth/KerberosSaslHelper.java ea2c689a4fb2e2ba6867b276044cbca0ac6899d3 
>   service/src/java/org/apache/hive/service/auth/PlainSaslHelper.java a161e1541e99fe30d7265a97bf196680d0fe905a 
> 
> 
> Diff: https://reviews.apache.org/r/58403/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>