[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-25 Thread felixcheung
Github user felixcheung commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
Ok, I will wait a day and merge if no other thought on this.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-24 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
ping @felixcheung @prabhjyotsingh 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-23 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
@felixcheung I think it is not necessary as I didn't find junit in 
https://github.com/apache/zeppelin/blob/master/zeppelin-distribution/src/bin_license/LICENSE


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-23 Thread felixcheung
Github user felixcheung commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
One question - I'm not sure if dependencies for test only code should be 
listed in LICENSE.
Otherwise LGTM.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-20 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
As mentioned in the PR description, for now you need to export SPARK_HOME 
and LIVY_HOME. I plan to merge with travis in the following PR. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-20 Thread prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
I just tried it on my local; so basically it skips all the test cases.

```
WARN [2016-10-20 18:17:05,017] ({main} 
LivyIntegrationTest.java[checkPreCondition]:66) - livy integration is skipped 
because LIVY_HOME is not set
 WARN [2016-10-20 18:17:05,018] ({main} 
LivyIntegrationTest.java[checkPreCondition]:66) - livy integration is skipped 
because LIVY_HOME is not set
 WARN [2016-10-20 18:17:05,019] ({main} 
LivyIntegrationTest.java[checkPreCondition]:66) - livy integration is skipped 
because LIVY_HOME is not set
 WARN [2016-10-20 18:17:05,008] ({main} 
LivyIntegrationTest.java[checkPreCondition]:66) - livy integration is skipped 
because LIVY_HOME is not set
```

Any plan for this to merge with travis ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-20 Thread prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
Otherwise yes, LGTM!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-20 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
@felixcheung @prabhjyotsingh I have published livy jars to OSSRH, and now I 
removed the livy jars. Please help review. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
Livy interpreter is not a trivial interpreter although it use rest api. The 
current unit test is not sufficient to guarantee the quality of livy 
interpreter. I have found some issues there, but before doing improvement and 
fix, I would like to commit this integration test, otherwise I have to manually 
test it after each fix and refactor. I know committing jars is not a good 
practise, but this is the only workaround I can think of.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread felixcheung
Github user felixcheung commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
As I have stated, I think these tests are good to have but worry about the 
added complicity for a source release having these jars in the "source", 
especially since these jars are not from an ASF project.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
Yes, that part I agree, it cannot be tested without jar, `org.mockito.Mock` 
will always mock. 

@Leemoonsoo  @jongyoul @felixcheung any thought about checking in jar(s) ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
mockit is for unit test, not for system test. It is just like 
[AbstractTestRestApi](https://github.com/apache/zeppelin/blob/master/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java)
 where we will launch real spark cluster and ZeppelinServer. Using mockit can 
not cover lots of cases in real enviroment (e.g. how can I test livy handle 
comment correctly (considering we are doing some preprocessing), how can I test 
SparkContext is shared correctly between %livy.spark and %livy.sql) . I know 
committing jars are not good practise, but these jars won't be available in 
public repository in short time. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
What I mean to say was, lets mock these till the time we get livy's jar 
from public repo, how do you think about that ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
@prabhjyotsingh  I dont' get it. This is integration test where I will 
launch a real livy server, how can I use mock to simulate that ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
Any thought about using `org.mockito.Mock`; example 
https://github.com/apache/zeppelin/blob/master/python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java#L78
 ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-10 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
@prabhjyotsingh These jars are not available in public maven repository. Do 
you have any better ideas ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-09 Thread prabhjyotsingh
Github user prabhjyotsingh commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
I too share same thought as @felixcheung, if we can avoid jar's, can we use 
`org.mockito.Mock` instead ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-10-07 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
reping @Leemoonsoo @prabhjyotsingh @bzz @jongyoul @felixcheung please help 
review. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-09-28 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
I have asked the livy community to publish these artifacts, suppose I can 
delete these livy artifacts in zeppelin after livy 0.3 is released. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-09-27 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
I am afraid this is the only workaround for now. Because these jars are not 
available in maven repository. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zeppelin issue #1462: ZEPPELIN-1477. Add Integration Test for LivyInterprete...

2016-09-27 Thread zjffdu
Github user zjffdu commented on the issue:

https://github.com/apache/zeppelin/pull/1462
  
@Leemoonsoo @prabhjyotsingh @felixcheung Please help review, the failed 
test is irrelevant, I have created 
[ZEPPELIN-1498](https://issues.apache.org/jira/browse/ZEPPELIN-1498) for it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---