hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Kolbasov <ako...@gmail.com>
Subject Re: Review Request 63586: Fix HIVE-17942: HiveAlterHandler should use the conf from HMS Handler
Date Thu, 09 Nov 2017 01:08:19 GMT

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



With this change, do we still need Config stored in the HiveAlterHandle itself? What is the
value of returning such config in getConf() Should  the new HiveAlterHandle extend COnfigurable?
Do we need setConf method?


itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreAlterColumnPar.java
Lines 37 (patched)
<https://reviews.apache.org/r/63586/#comment267971>

    It is unclear from the test that that's what is testing, so it would be good to explain
how your test actually tests for this.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
Line 109 (original), 109 (patched)
<https://reviews.apache.org/r/63586/#comment267972>

    Can you add a comment at the top of the class explaining that only the handler's config
should be used and never the local stashed config?
    
    Do we need to support local config at all?


- Alexander Kolbasov


On Nov. 8, 2017, 7:17 p.m., Janaki Lahorani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63586/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2017, 7:17 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Andrew Sherman, Sahil Takiar, and Vihang
Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HMS handler thread local will have the configuration changes from the user local only
to that connection.  HiveAlterHandler should use the thread local to pick up user's configuration
changes.
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStoreAlterColumnPar.java
PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
921cfc00343807179340fbdf40f21e2a46d936ab 
> 
> 
> Diff: https://reviews.apache.org/r/63586/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Janaki Lahorani
> 
>


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