----------------------------------------------------------- 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 > >
