hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "iryoung jeong" <iryo...@gmail.com>
Subject Re: Review Request: HIVE-2489 Add configuration properties for HiveServer and HiveMetaStore port numbers and fix related unittest
Date Tue, 15 Nov 2011 17:26:17 GMT


> On 2011-11-15 15:52:04, Ashutosh Chauhan wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 249
> > <https://reviews.apache.org/r/2833/diff/1/?file=58385#file58385line249>
> >
> >     Typo. Default value is 9083, not 9803.

oops. I'll fix right now. 


> On 2011-11-15 15:52:04, Ashutosh Chauhan wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, line 3696
> > <https://reviews.apache.org/r/2833/diff/1/?file=58386#file58386line3696>
> >
> >     Why are you recreating HiveConf object here ?

Without recreating, TestMetaStoreAuthorization.testMetaStoreAuthorization() is failed.

I thought there're issues  HiveConf initialization  which can cause the problem.
(To be honest, I don't know all the underlying issues relating to metastore tests.)
But this workaround can make the tests pass.

if there's better way, please let me know.

Or, preserve previous interface not to change legacy tests?
while I was making this patch, I wondered that updating legacy tests is out of HIVE-2489 scope
or not.


- iryoung


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


On 2011-11-15 15:19:49, iryoung jeong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2833/
> -----------------------------------------------------------
> 
> (Updated 2011-11-15 15:19:49)
> 
> 
> Review request for hive and Carl Steinbach.
> 
> 
> Summary
> -------
> 
> There're 3 ways to set port now - using default port, from cli(-p or old style), HIVE_PORT/METASTORE_PORT
system env.
> And after this issue is fixed, there'll be another way to configure port.
> 
> So, it's necessary cleaning up these.
> 
> Previous setting port precedence looks like this "default port < system env < CLI
option"
> (if higher one is set, then use it)
> 
> and, now it'll be "default port < system env < HiveConf < CLI option"
> 
> IMO, all other settings should be finally collected into a HiveConf object. So I implemented
this way for HiveMetaStore.
> 
> But in HiveServer, some configurations are only available through CLI options, so I chose
applying port number from HiveConf to previous CLI object.(which is required minimum code
change, and I thought moving other settings to HiveConf is out of scope of this issue)
> 
> 
> This addresses bug HIVE-2489.
>     https://issues.apache.org/jira/browse/HIVE-2489
> 
> 
> Diffs
> -----
> 
>   shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java aadb34d 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 2a52fd1 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java ffce4fe 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java cd533b6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestMetaStoreAuthorization.java
a089269 
>   service/src/java/org/apache/hadoop/hive/service/HiveServer.java a2d599f 
> 
> Diff: https://reviews.apache.org/r/2833/diff
> 
> 
> Testing
> -------
> 
> previous tests passed
> 
> 
> Thanks,
> 
> iryoung
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message