----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24986/#review52030 -----------------------------------------------------------
Hi, Thank you for the update! I have commented below! Cheers! Brock ql/src/java/org/apache/hadoop/hive/ql/processors/ReloadProcessor.java <https://reviews.apache.org/r/24986/#comment90742> CommandProcessorResponse.create only sends the message. Can we log this exception on the server as well. ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java <https://reviews.apache.org/r/24986/#comment90754> Can you switch this to private final? ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java <https://reviews.apache.org/r/24986/#comment90743> Let's add Exception.toString() to this message: String msg = "...: " + e; ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java <https://reviews.apache.org/r/24986/#comment90744> String msg = "...: " + e; ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java <https://reviews.apache.org/r/24986/#comment90748> This can cause issues if tests run currently on the same host. Let's use: Files.createTmpDir() ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java <https://reviews.apache.org/r/24986/#comment90746> Hi, to give devs a better better error message in the case of failure, how about: Assert.assertEquals(Sets.newHashSet(jarFileName1, jarFileName2), jars); ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java <https://reviews.apache.org/r/24986/#comment90747> Let's import Assert or Assert.fail. ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java <https://reviews.apache.org/r/24986/#comment90749> Files.createTmpDir() ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java <https://reviews.apache.org/r/24986/#comment90752> Hi, Do we need to use threads here? Threading often makes tests falky. Perhaps we can do this without threading? ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java <https://reviews.apache.org/r/24986/#comment90750> I don't think this failure will be caught since it's thrown in a seperate thread. Can we change this code so if an exception is thrown in these threads the main body of the test knows? ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java <https://reviews.apache.org/r/24986/#comment90751> Above we use th.getContextLoader(). Why do we use that above and then ss.getConf().getClassLoader()? Here? service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java <https://reviews.apache.org/r/24986/#comment90753> If this fails, we should throw this exception. - Brock Noland On Sept. 1, 2014, 7:16 a.m., cheng xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24986/ > ----------------------------------------------------------- > > (Updated Sept. 1, 2014, 7:16 a.m.) > > > Review request for hive. > > > Bugs: HIVE-7553 > https://issues.apache.org/jira/browse/HIVE-7553 > > > Repository: hive-git > > > Description > ------- > > UT fix > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 7f4afd9d64aff18329e7850342855aade42e21f5 > hcatalog/core/src/main/java/org/apache/hive/hcatalog/common/HCatUtil.java > 93a03adeab7ba3c3c91344955d303e4252005239 > > hcatalog/webhcat/java-client/src/main/java/org/apache/hive/hcatalog/api/HCatClient.java > f25039dcf55b3b24bbf8dcba05855665a1c7f3b0 > ql/src/java/org/apache/hadoop/hive/ql/exec/DefaultFetchFormatter.java > 5924bcf1f55dc4c2dd06f312f929047b7df9de55 > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java > 0c6a3d44ef1f796778768421dc02f8bf3ede6a8c > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionTask.java > bd45df1a401d1adb009e953d08205c7d5c2d5de2 > ql/src/java/org/apache/hadoop/hive/ql/exec/ListSinkOperator.java > dcc19f70644c561e17df8c8660ca62805465f1d6 > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java > 76fee612a583cdc2c632d27932623521b735e768 > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java > eb2851b2c5fa52e0f555b3d8d1beea5d1ac3b225 > ql/src/java/org/apache/hadoop/hive/ql/hooks/HookUtils.java > 3f474f846c7af5f1f65f1c14f3ce51308f1279d4 > ql/src/java/org/apache/hadoop/hive/ql/io/HivePassThroughOutputFormat.java > 0962cadce0d515e046371d0a816f4efd70b8eef7 > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java > 9051ba6d80e619ddbb6c27bb161e1e7a5cdb08a5 > ql/src/java/org/apache/hadoop/hive/ql/metadata/Partition.java > edec1b734fb2f015902fd5e1c8afd5acdf4cb3bf > ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java > 2f13ac2e30195a25844a25e9ec8a7c42ed99b75c > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java > b15aedc15d8cd0979aced6ff4c9e87606576f0a3 > ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java > d86df453cd7686627940ade62c0fd72f1636dd0b > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java > 0a1c660b4bbd46d8410e646270b23c99a4de8b7e > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > b05d3b48ec014e4dc8026bb5f6615f62da0e2210 > ql/src/java/org/apache/hadoop/hive/ql/plan/AggregationDesc.java > 17eeae1a3435fceb4b57325675c58b599e0973ea > ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java > 930acbc98e81f8d421cee1170659d8b7a427fe7d > ql/src/java/org/apache/hadoop/hive/ql/plan/TableDesc.java > 39f1793aaa5bed8a494883cac516ad314be951f4 > > ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java > 0d237f01a248a65b4092eb7202fe30eebf27be82 > ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java > f5bc427a5834860441f21bfc72e175c6a1cf877f > ql/src/java/org/apache/hadoop/hive/ql/processors/ReloadProcessor.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java > 9798cf3f537a27d1f828f8139790c62c5945c366 > ql/src/java/org/apache/hadoop/hive/ql/stats/StatsFactory.java > e247184b7d95c85fd3e12432e7eb75eb1e2a0b68 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBridge.java > 959007a54b335bb0bdef0256f60e6cbc65798dc7 > ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java > 7fd6c172d18e0d68dc0568848a5fad255a0a9188 > ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java > ef0052f5763922d50986f127c416af5eaa6ae30d > ql/src/test/resources/SessionStateTest.jar.v1 PRE-CREATION > ql/src/test/resources/SessionStateTest.jar.v2 PRE-CREATION > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > bc0a02c1df7f9fdb848d5f078e94a663a579e571 > > Diff: https://reviews.apache.org/r/24986/diff/ > > > Testing > ------- > > > Thanks, > > cheng xu > >
