ZEPPELIN-925 Merge HiveInterpreter into JDBCInterpreter #943

jongyoul · 2016-06-01T06:57:59Z

What is this PR for?

This removes hive module and adds example setting for using Hive in a JdbcInterpreter by using loading dynamic dependencies. It reduces Zeppelin's binary size.

There's no codes' modification except removing hive directory and remove the module from pom.xml

What type of PR is it?

[Feature]

Todos

- Remove hive module
- Add an example for using Hive in JDBC

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-925

How should this be tested?

Set the interpreter properties and test it

Screenshots (if appropriate)

Questions:

Does the licenses files need update? No
Is there breaking changes for older versions? No
Does this needs documentation? No

jongyoul · 2016-06-01T07:09:34Z

@minahlee Could you please review this PR, especially setting dependencies?

jongyoul · 2016-06-01T07:51:55Z

That failure is irrelevant to this PR.

Results :

Failed tests: 
  ZeppelinSparkClusterTest.pySparkDepLoaderTest:231 expected:<FINISHED> but was:<ERROR>

Tests run: 39, Failures: 1, Errors: 0, Skipped: 0

jongyoul · 2016-06-01T07:58:18Z

re-trigger

bzz · 2016-06-01T09:15:09Z

CI is failing by this test ZeppelinSparkClusterTest.sparkRTest, and I do not think we have seen it failing before.

Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 168.646 sec <<< FAILURE! - in org.apache.zeppelin.rest.ZeppelinSparkClusterTest
sparkRTest(org.apache.zeppelin.rest.ZeppelinSparkClusterTest)  Time elapsed: 11.03 sec  <<< FAILURE!
java.lang.AssertionError: expected:<FINISHED> but was:<ERROR>
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.failNotEquals(Assert.java:743)
    at org.junit.Assert.assertEquals(Assert.java:118)
    at org.junit.Assert.assertEquals(Assert.java:144)
    at org.apache.zeppelin.rest.ZeppelinSparkClusterTest.sparkRTest(ZeppelinSparkClusterTest.java:104)

@jongyoul could you please create a new JIRA issue with label flaky-test for it?

Otherwise, converging interpreters to JDBC looks great to me!

May be we should keep in docs somewhere a notice for Hive users, something like "If you want to use Hive - please use it though JDBC interpreter, configuring it like this ...", how do you think?

So if somebody wants to use Zeppelin with Hive - he should be able to understand from the website\docs that it is indeed supported use-case, and he should be using JDBC (and not a special interpreter \w ThriftClient)

jongyoul · 2016-06-01T14:44:28Z

@bzz What you have seen is different from the first error. Have you ever seen the first one before? I'll make a new issue for second error with flaky-test tag.

And it's good to remain documentation about hive until - may be - 0.7.0 to guide that users use a JDBC interpreter to use hive. I'll update hive.md again.

jongyoul · 2016-06-01T15:34:51Z

@bzz I made a https://issues.apache.org/jira/browse/ZEPPELIN-936

minahlee · 2016-06-01T21:34:00Z

@jongyoul changes looks good to me. About the documentation I think same as @bzz for the users who were already using hive interpreter. I think it would be good to keep hive.md document and mention that hive interpreter is merged into master and refer the jdbc interpreter doc link.

jongyoul · 2016-06-02T04:45:31Z

@bzz @minahlee Please check it finally. And I email {dev,user}@ to announce this change.

felixcheung · 2016-06-02T05:14:07Z

docs/interpreter/hive.md

-ORDER BY count ${order=DESC,DESC|ASC}
-LIMIT ${limit=10};
-```
+Hive Interpreter is merged into JDBC Interpreter. You can see JDBC Interpreter [here](./jdbc.html)


I think it is common to use Zeppelin with hive and the configuration values and example might still be useful in documentation?

@felixcheung I've also update an example to use hive in JDBC interpreter here Do you think it's better to remain them in this page? I also agree with you. I'll update it.

jongyoul · 2016-06-02T06:08:08Z

@felixcheung How about it? And another option is to remain hive.md originally and add message of moving it into JDBC Interpreter on the top of page. Which do you think is better?

AhyoungRyu · 2016-06-02T06:20:55Z

docs/interpreter/hive.md

+   </tr>
+ </table>
+### Dependencies
+ <table>


@jongyoul Could you change this line to <table class="table-configuration"> instead of <table> ?

jongyoul · 2016-06-02T08:34:14Z

re-trigger

bzz · 2016-06-02T09:36:13Z

docs/interpreter/hive.md

-FROM retail_demo.order_lineitems_pxf
-GROUP BY ${group_by=product_id,product_id|product_name|customer_id|store_id}
-ORDER BY count ${order=DESC,DESC|ASC}
-LIMIT ${limit=10};


Does this example stoped working with the JDBC interpreter?
Otherwise it would be nice to keep it too, as that is something valuable to a user.

@bzz JDBC Interpreter covers all of Hive Interpreter features. Then, how about adding a link and messages like that This is deprecated. You'd better use JDBC Interpreter?

since the dynamic form example isn't hive specific, maybe it is ok not to have it. though i agree it's generally nicer to have more documentation ;)

@bzz @felixcheung OK. I'll revert hive.md and add some notification about how to move hive to jdbc. I also think it would be better and less confusion to existing users.

jongyoul · 2016-06-07T06:40:07Z

Update documentation and rebase it from current master

jongyoul · 2016-06-08T03:55:52Z

@felixcheung @bzz @minahlee @AhyoungRyu I've reverted hive.md and updated documentation. Please check it finally.

vgmartinez · 2016-06-08T07:33:37Z

Hi @jongyoul, great job with all integrations in the jdbc interpreter.
What do you think, about putting hive dependence into the pom and so it is not necessary add the jar in the classpath.

jongyoul · 2016-06-08T07:37:06Z

@vgmartinez Now, Zeppelin enable load dependencies dynamically, thus if you set dependencies in your interpreter tab, it works. Thus, we don't have to include any dependencies any more. See an example below:

vgmartinez · 2016-06-08T07:41:53Z

@jongyoul ok ok, all external dependencies loaded them that way. I have been a time out...;)

AhyoungRyu · 2016-06-08T07:49:48Z

@jongyoul Yeah looks good :)

jongyoul · 2016-06-08T15:14:54Z

Merging there's no more discussion

This reverts commit e6822b6.

jongyoul closed this Jun 1, 2016

jongyoul reopened this Jun 1, 2016

felixcheung reviewed Jun 2, 2016
View reviewed changes

AhyoungRyu reviewed Jun 2, 2016
View reviewed changes

jongyoul closed this Jun 2, 2016

jongyoul reopened this Jun 2, 2016

bzz reviewed Jun 2, 2016
View reviewed changes

jongyoul force-pushed the ZEPPELIN-925 branch 2 times, most recently from 49c2152 to b8821f9 Compare June 7, 2016 06:39

jongyoul closed this Jun 7, 2016

jongyoul reopened this Jun 7, 2016

jongyoul force-pushed the ZEPPELIN-925 branch from 3fbe981 to 1ff5d9a Compare June 9, 2016 05:42

Update example for hive in jdbc.md

a38dc28

jongyoul added 8 commits June 9, 2016 14:44

Remove hive module and its directory

cdf1cfc

Remove hive.md from docs/interpreter

9c58640

Updated Documentation

f720ed5

Deleted hive module from pom.xml

f6573e3

Revert "Remove hive.md from docs/interpreter"

6aa9c7b

This reverts commit e6822b6.

Update hive.md

f6b94d6

Updated style of jdbc.md

45ec667

Fixed typo

eb50c78

jongyoul force-pushed the ZEPPELIN-925 branch from 1ff5d9a to eb50c78 Compare June 9, 2016 05:45

asfgit closed this in 43baa0a Jun 9, 2016

bzz mentioned this pull request Aug 3, 2016

[ZEPPELIN-1261] Bug fix in z.show() for matplotlib graphs #1267

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZEPPELIN-925 Merge HiveInterpreter into JDBCInterpreter #943

ZEPPELIN-925 Merge HiveInterpreter into JDBCInterpreter #943

jongyoul commented Jun 1, 2016 •

edited

jongyoul commented Jun 1, 2016

jongyoul commented Jun 1, 2016

jongyoul commented Jun 1, 2016

bzz commented Jun 1, 2016 •

edited

jongyoul commented Jun 1, 2016

jongyoul commented Jun 1, 2016

minahlee commented Jun 1, 2016

jongyoul commented Jun 2, 2016

felixcheung Jun 2, 2016

jongyoul Jun 2, 2016 •

edited

jongyoul commented Jun 2, 2016 •

edited

AhyoungRyu Jun 2, 2016

jongyoul commented Jun 2, 2016

bzz Jun 2, 2016

jongyoul Jun 2, 2016

felixcheung Jun 3, 2016

jongyoul Jun 7, 2016

jongyoul commented Jun 7, 2016

jongyoul commented Jun 8, 2016

vgmartinez commented Jun 8, 2016

jongyoul commented Jun 8, 2016

vgmartinez commented Jun 8, 2016

AhyoungRyu commented Jun 8, 2016

jongyoul commented Jun 8, 2016

ZEPPELIN-925 Merge HiveInterpreter into JDBCInterpreter #943

ZEPPELIN-925 Merge HiveInterpreter into JDBCInterpreter #943

Conversation

jongyoul commented Jun 1, 2016 • edited

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

jongyoul commented Jun 1, 2016

jongyoul commented Jun 1, 2016

jongyoul commented Jun 1, 2016

bzz commented Jun 1, 2016 • edited

jongyoul commented Jun 1, 2016

jongyoul commented Jun 1, 2016

minahlee commented Jun 1, 2016

jongyoul commented Jun 2, 2016

felixcheung Jun 2, 2016

Choose a reason for hiding this comment

jongyoul Jun 2, 2016 • edited

Choose a reason for hiding this comment

jongyoul commented Jun 2, 2016 • edited

AhyoungRyu Jun 2, 2016

Choose a reason for hiding this comment

jongyoul commented Jun 2, 2016

bzz Jun 2, 2016

Choose a reason for hiding this comment

jongyoul Jun 2, 2016

Choose a reason for hiding this comment

felixcheung Jun 3, 2016

Choose a reason for hiding this comment

jongyoul Jun 7, 2016

Choose a reason for hiding this comment

jongyoul commented Jun 7, 2016

jongyoul commented Jun 8, 2016

vgmartinez commented Jun 8, 2016

jongyoul commented Jun 8, 2016

vgmartinez commented Jun 8, 2016

AhyoungRyu commented Jun 8, 2016

jongyoul commented Jun 8, 2016

jongyoul commented Jun 1, 2016 •

edited

bzz commented Jun 1, 2016 •

edited

jongyoul Jun 2, 2016 •

edited

jongyoul commented Jun 2, 2016 •

edited