----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57343/#review168162 -----------------------------------------------------------
itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java Lines 44-45 (patched) <https://reviews.apache.org/r/57343/#comment240309> Nit : I see that necessary imports are already there (or could be added) is there a reason to declare it this way? itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java Lines 71 (patched) <https://reviews.apache.org/r/57343/#comment240310> Nit : Spellcheck - getExpectedOutputFile() itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java Lines 195-197 (patched) <https://reviews.apache.org/r/57343/#comment240311> Is this pre-existing code? Is there a way to make this more robust? A change in this log message in the source code will fail all the BeeLine tests. itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java Lines 259-264 (patched) <https://reviews.apache.org/r/57343/#comment240313> Use File.separator instead of "/" to make it OS agnostic itests/util/src/main/java/org/apache/hive/beeline/qfile/QFileBeeLineClient.java Lines 71-72 (patched) <https://reviews.apache.org/r/57343/#comment240314> Curious to understand why do we need to create one database for each qFile? Also, may be a good idea to refactor these lines into a beforeExecute() method itests/util/src/main/java/org/apache/hive/beeline/qfile/QFileBeeLineClient.java Lines 80-83 (patched) <https://reviews.apache.org/r/57343/#comment240315> Can we refactor this to a afterExecute() method itests/util/src/main/java/org/apache/hive/beeline/qfile/QFileBeeLineClient.java Lines 94-95 (patched) <https://reviews.apache.org/r/57343/#comment240316> Do we need this in a finally block? itests/util/src/main/java/org/apache/hive/beeline/qfile/package-info.java Lines 22 (patched) <https://reviews.apache.org/r/57343/#comment240317> Not sure I understand this file.. - Vihang Karajgaonkar On March 7, 2017, 3:14 p.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57343/ > ----------------------------------------------------------- > > (Updated March 7, 2017, 3:14 p.m.) > > > Review request for hive, Zoltan Haindrich, Naveen Gangam, Sergio Pena, Vihang > Karajgaonkar, and Barna Zsombor Klara. > > > Bugs: HIVE-16127 > https://issues.apache.org/jira/browse/HIVE-16127 > > > Repository: hive-git > > > Description > ------- > > Refactored the QFileClient: > - Moved to itest/util > - Separated QFile specific code parts (file path, and filtering) > - Separated BeeLineClient specific code (execution of the queries, commands) > - Created factories for QFile, and Client, so after initialization they can > be reused during multiple tests > - Separated init script run from actual test run, so multiple tests could be > run against a single MiniHS2 instance > - Removed unused property > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/util/QFileClient.java d306b7f > itests/src/test/resources/testconfiguration.properties b01ebd8 > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java > aba1fde > itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java > PRE-CREATION > > itests/util/src/main/java/org/apache/hive/beeline/qfile/QFileBeeLineClient.java > PRE-CREATION > itests/util/src/main/java/org/apache/hive/beeline/qfile/package-info.java > PRE-CREATION > ql/src/test/results/clientpositive/beeline/drop_with_concurrency.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/57343/diff/1/ > > > Testing > ------- > > Used the previosly generated test to validate that the result is not changed. > Added one more test to check the separation is done correctly. > > > Thanks, > > Peter Vary > >
