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


Fix it, then Ship it!




Just some random thoughts in the morning


itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCliConfig.java
 (lines 80 - 84)
<https://reviews.apache.org/r/51468/#comment214183>

    It is mostly just a question, which appeared to me in the morning, waking 
up:
    - There are several other places which are using QTestUtil's query running 
framework - do any of them uses these properties? If so, then it might be good 
idea to put these defaults to the QTestUtil constructor, so these initialized 
there as well, and could be run from browser too.
    
    I could see several valid answers why you did not do this, like:
    - These are only used by qtest queries
    - We concentrate on the qtests now, and do not now enough of the other test 
to do this change
    
    This morning ideas are sometimes very good, sometimes not so much, just 
"stored" here to check when realy aweaken :)
    
    Another one of these ideas:
    The next develeper, who adds a new variable, he might forget to add it 
here, so the test will only work with maven, and not with eclipse, or idea. I 
think we should add a comment to the pom.xml, to remaind him to add a default 
value here as well. For example after this comment:
    
        <!-- required by QTestUtil -->
        
<test.data.files>${basedir}/${hive.path.to.root}/data/files</test.data.files>
    
    What do you think?


Thanks for your hard work!
I really appreciate you working on stuff even on the weekends!

- Peter Vary


On Aug. 26, 2016, 10:13 p.m., Zoltan Haindrich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51468/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2016, 10:13 p.m.)
> 
> 
> Review request for hive, Balint Molnar, Lefty Leverenz, and Prasanth_J.
> 
> 
> Bugs: HIVE-14532
>     https://issues.apache.org/jira/browse/HIVE-14532
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> how to execute qtests for ide wikipage draft:
> http://hastebin.com/paxicutive.vhdl
> 
> the patch itself contains:
> 
> * some automatic property settings to configure qtest related things to be 
> able to execute
> * maven profile to avoid shading plugin invocation during IDE project 
> generation
> * some test.src.tables related changes
> 
> 
> Diffs
> -----
> 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCliConfig.java
>  efbd4657f22e856b9c9ba5f74472ad5fd9f9a5b5 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CliConfigs.java 
> 319a205225c67f123ba35a2811ee117650d46dc3 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 
> 4d4a929c159c61f9f4af3238d4b7baff146d346e 
>   jdbc/pom.xml b29739b3f8577c6e363b5c8ee39b63e53a17c907 
>   pom.xml 9ed1c19f3e312ee1f1d9932ee2ba228589a7cb49 
>   ql/pom.xml 02ddb805a228ed23694c8a81953dd2400d7308c6 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/EnforceReadOnlyTables.java 
> 4569ed5e41f2f4db8f3d539d7cfb693c442ba910 
> 
> Diff: https://reviews.apache.org/r/51468/diff/
> 
> 
> Testing
> -------
> 
> I've tested the draft using eclipse:
> 3.0 -> 3.1 -> TestCliDriver(combine2,alter_concatenate_indexed_table)
> 3.0 -> 3.2 -> TestCliDriver(combine2)
> 
> I think this should work with idea too...but someone should possibly check it 
> ;)
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>

Reply via email to