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


Ship it!




Ship It!

- Barna Zsombor Klara


On Oct. 5, 2017, 4:39 p.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62782/
> -----------------------------------------------------------
> 
> (Updated Oct. 5, 2017, 4:39 p.m.)
> 
> 
> Review request for hive and Barna Zsombor Klara.
> 
> 
> Bugs: HIVE-17706
>     https://issues.apache.org/jira/browse/HIVE-17706
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> - Added a cleanup method which removed unkonwn databases, tables and view. 
> This makes it is possible to run the tests sequentially using 'default' 
> database.
> - Renamed the test.rewrite.source.tables parameter to 
> test.beeline.run.parallel so it makes more sense.
> - Copied some masking from QTestUtil
> - Enhanced the table name rewriting regexp a little, so mixed case tablenames 
> are kept as it is
> - In the QFile made it possible to not call create/drop database command, if 
> not needed.
> 
> I would value any advice where the cleanup logic should be kept.
> - I this solution when the tests are parallel then the cleanup is in 
> QFileBeeLineClient (create/drop database), when the tests are sequential then 
> the cleanup is in the CoreBeeLineDriver.
> 
> Would it be a good idea to move every cleanup related stuff to 
> CoreBeeLineDriver? Like:
> - QFileBeeLineClient.beforeExecute
> - QFileBeeLineClient.afterExecute
> Both of these are need QFile specific info, and an existing BeeLineClient, 
> which we currently do not have in hand in the CoreBeeLineDriver, and more 
> refactoring is needed
> 
> Or would it be a good idea to move every cleanup related stuff to 
> QFileBeeLineClient? Like:
> - CoreBeeLineDriver.runCleanup
> This is really cleanup stuff
> 
> Or we should leave as it is :)
> 
> Any comments are welcome.
> Thanks,
> Peter
> 
> 
> Diffs
> -----
> 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java
>  9dfc253 
>   itests/util/src/main/java/org/apache/hive/beeline/QFile.java e70ac38 
>   itests/util/src/main/java/org/apache/hive/beeline/QFileBeeLineClient.java 
> 2f91834 
> 
> 
> Diff: https://reviews.apache.org/r/62782/diff/2/
> 
> 
> Testing
> -------
> 
> Run the tests manually with, and without the test.beeline.run.parallel 
> parameter
> 
> 
> Thanks,
> 
> Peter Vary
> 
>

Reply via email to