> On Aug. 28, 2016, 3:07 p.m., Peter Vary wrote:
> > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/AbstractCliConfig.java,
> > lines 80-84
> > <https://reviews.apache.org/r/51468/diff/1/?file=1487189#file1487189line80>
> >
> > 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?
>
> Zoltan Haindrich wrote:
> i think most of these extra properties should be removed later and/or use
> some convention instead of setting them(warehousedir/tmpdir)
>
> test.data.files - is a property which has no real alternative
> value...that directory contains so many specific files that you can't really
> change it...
My bad again. My comment was not clear enough :(
My second comment was not about the concrete property from the pom.xml, it was
more about the place where every QTestUtil related property is set. If I have
provided longer sniplet it might be more clear what I wanted to say:
<!-- required by QTestUtil -->
<test.data.files>${basedir}/${hive.path.to.root}/data/files</test.data.files>
<test.data.dir>${basedir}/${hive.path.to.root}/data/files</test.data.dir>
<test.tmp.dir>${test.tmp.dir}</test.tmp.dir>
<test.tmp.dir.uri>${test.tmp.dir.uri}</test.tmp.dir.uri>
<test.dfs.mkdir>${test.dfs.mkdir}</test.dfs.mkdir>
<test.output.overwrite>${test.output.overwrite}</test.output.overwrite>
<test.warehouse.dir>${test.warehouse.scheme}${test.warehouse.dir}</test.warehouse.dir>
<java.net.preferIPv4Stack>true</java.net.preferIPv4Stack>
If we remind the developer in the comment, that the properties should be set in
AbstractCliConfig as well; if a value is changed, or new value is added, then
it is less likely that we end up with new tests which could be run with maven,
but could not be run in IDE. It would be something like this:
<!-- required by QTestUtil - please set them in AbstractCliConfig
too so the test could be run form IDE -->
Of course the patch is great without this too, just wanted to correct my
communication error.
Thanks for your patience,
Peter
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51468/#review147100
-----------------------------------------------------------
On Sept. 7, 2016, 8:39 p.m., Zoltan Haindrich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51468/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2016, 8:39 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:
> v1: 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 - to void classpath/compile errors i've encountered
> * some test.src.tables related changes - defaulting is now done at a
> different place
> * eclipse is ok for me
>
> patch#2 here is patch#3 in the jira
>
>
> Diffs
> -----
>
> itests/hive-jmh/pom.xml f1417fd7e8b70a7a3dd255c25d3909013f02df67
>
> 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
> 69c4974105c6b47cc8e8051cd980274c9c381775
> itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java
> 4d4a929c159c61f9f4af3238d4b7baff146d346e
> jdbc/pom.xml b29739b3f8577c6e363b5c8ee39b63e53a17c907
> pom.xml 4c41200ffc8e2c9a1d207f8676f252b94e80e4fd
> ql/pom.xml 02ddb805a228ed23694c8a81953dd2400d7308c6
>
> 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)
>
> idea:
> althrough I was able to create a few working setups...I think it will need
> more work
>
>
> Thanks,
>
> Zoltan Haindrich
>
>
