You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by rawkintrevo <gi...@git.apache.org> on 2016/03/23 20:03:55 UTC
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-658] Companion object b...
GitHub user rawkintrevo opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/794
[ZEPPELIN-658] Companion object bug fix for Flink Interpretter
### What is this PR for?
This implements the solution presented for the Spark interpreter in PR-780 for the Flink interpreter. The Issue didn't explicitly specify the Spark interpreter so closing the issue when only fixing for Spark was somewhat premature.
### What type of PR is it?
Bug Fix
### Todos
* [ ] - Fix line splitting (Flink)
* [ ] - Add tests (Flink)
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-658
* https://issues.apache.org/jira/browse/ZEPPELIN-747
### How should this be tested?
Add a new paragraph with Scala/Flink interpreter and define a class with its companion object.
See bug for more details
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rawkintrevo/incubator-zeppelin flink-companion-objects
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/794.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #794
----
commit bb8da6cc587bcad35d05c0beca14dad409df3808
Author: Trevor Grant <tr...@gmail.com>
Date: 2016-03-23T18:58:57Z
[ZEPPELIN-658] Companion object bug fix for Flink Interpretter
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-202517074
@rawkintrevo Please feel free to confine this change if it would help.
We could follow up separately for Ignite.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201269846
I was getting test errors earlier because I copy/pasted to fast and didn't change the name of the interpreter context variable as I went (or changed it to the wrong thing).
I looked through the test profile and can't find the specified error. If I have missed it could you please reference a line number in the build logs.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-203979194
Change looks good to me. Thanks @rawkintrevo for the nice patch.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201433744
3622.1 Failed on
assertTrue(result.message().contains(sizeVal + ": Int = " + ignite.cluster().nodes().size()));
Which I didn't touch (?)
3622.3 Had multiple failures in
Zeppelin: Server
ZeppelinSparkClusterTest.pySparkDepLoaderTest:167->getSparkVersionNumber:220 expected:<FINISHED> but was:<ERROR>
ZeppelinSparkClusterTest.pySparkAutoConvertOptionTest:112->getSparkVersionNumber:220 expected:<FINISHED> but was:<ERROR>
ZeppelinSparkClusterTest.basicRDDTransformationAndActionTest:81 expected:<FINISHED> but was:<ERROR>
ZeppelinSparkClusterTest.pySparkTest:90->getSparkVersionNumber:220 expected:<FINISHED> but was:<ERROR>
ZeppelinSparkClusterTest.zRunTest:152 expected:<FINISHED> but was:<ERROR>
3622.7
Also failed in Zeppelin: Server, but at different tests?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-658] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-200554562
this has failed twice on npm install... ?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201441447
That seems flaky test. Could you re-trigger CI once? close-open pullrequest or adding a commit will trigger CI.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/794
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201649645
I ran tests on my machine my test stopped
```
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.267 sec - in org.apache.zeppelin.ignite.IgniteSqlInterpreterTest
Results :
Failed tests:
IgniteInterpreterTest.testInterpret:102 null
Tests run: 7, Failures: 1, Errors: 0, Skipped: 0
```
`IgniteInterpreterTest.testInterpret` seems okay itself.
But when it runs after other tests in `IgniteInterpreterTest`, [`result.message()` at line 102](https://github.com/apache/incubator-zeppelin/pull/794/files#diff-3f8aa43c71703e591afbf0f0bb9174fdR102) returns result of previous test and it fails. This is strange. @rawkintrevo Do you have any idea on this behavior?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201319759
Please take a look line number around 12036 of job 3610.1
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-202463668
I really don't. I don't know much of anything about Ignite. I really was only doing this for Flink, but Felix asked that I apply the fix to Scalding/Ignite as well since the fix was essentially the same. While the code for the interpreters are essentially the same, there is a lot more variance in the structure and syntax of the testing files which has been causing all of these issues.
As I read the code, its just asking how many clusters and nodes exist? Is there someone out there that knows Ignite that could weigh in?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
GitHub user rawkintrevo reopened a pull request:
https://github.com/apache/incubator-zeppelin/pull/794
[ZEPPELIN-760] Companion object bug fix for Flink Interpretter
### What is this PR for?
This implements the solution presented for the Spark interpreter in PR-780 for the Flink, Scalding, and Ignite interpreters.
### What type of PR is it?
Improvement
### Todos
* [x] - Fix line splitting
* [x] - Add tests
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-658
* https://issues.apache.org/jira/browse/ZEPPELIN-747
### How should this be tested?
Add a new paragraph with each interpreter and define a class with its companion object.
See bug ZEPPELIN-658 for more details
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rawkintrevo/incubator-zeppelin flink-companion-objects
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/794.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #794
----
commit 994312c80745fd1d23db09396a5704d77d87177d
Author: Trevor Grant <tr...@gmail.com>
Date: 2016-03-23T18:58:57Z
[ZEPPELIN-658] Companion object bug fix for Flink Interpretter
push torestart build
[ZEPPELIN-760] Companion objects in Flink, Scalding, and Ignite interpreters
[ZEPPELIN-760] Fixed Tests
[ZEPPELIN-760] Fixed Ignite Test
[ZEPPELIN-760] Fixed Ignite Test
commit ff086c77a954433e65e0b3f850ab2503d029d212
Author: Trevor Grant <tr...@gmail.com>
Date: 2016-03-25T15:38:34Z
[ZEPPELIN-760] Dropped duplicate Scalding Test
commit b37adbbec4e14ef7264ac750a3d7a4cdc04599e8
Author: Trevor Grant <tr...@gmail.com>
Date: 2016-03-25T16:05:20Z
[ZEPPELIN-760] Dropped redundent Flink Test
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
GitHub user rawkintrevo reopened a pull request:
https://github.com/apache/incubator-zeppelin/pull/794
[ZEPPELIN-760] Companion object bug fix for Flink Interpretter
### What is this PR for?
This implements the solution presented for the Spark interpreter in PR-780 for the Flink, Scalding, and Ignite interpreters.
### What type of PR is it?
Improvement
### Todos
* [x] - Fix line splitting
* [x] - Add tests
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-658
* https://issues.apache.org/jira/browse/ZEPPELIN-747
### How should this be tested?
Add a new paragraph with each interpreter and define a class with its companion object.
See bug ZEPPELIN-658 for more details
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rawkintrevo/incubator-zeppelin flink-companion-objects
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/794.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #794
----
commit 994312c80745fd1d23db09396a5704d77d87177d
Author: Trevor Grant <tr...@gmail.com>
Date: 2016-03-23T18:58:57Z
[ZEPPELIN-658] Companion object bug fix for Flink Interpretter
push torestart build
[ZEPPELIN-760] Companion objects in Flink, Scalding, and Ignite interpreters
[ZEPPELIN-760] Fixed Tests
[ZEPPELIN-760] Fixed Ignite Test
[ZEPPELIN-760] Fixed Ignite Test
commit ff086c77a954433e65e0b3f850ab2503d029d212
Author: Trevor Grant <tr...@gmail.com>
Date: 2016-03-25T15:38:34Z
[ZEPPELIN-760] Dropped duplicate Scalding Test
commit b37adbbec4e14ef7264ac750a3d7a4cdc04599e8
Author: Trevor Grant <tr...@gmail.com>
Date: 2016-03-25T16:05:20Z
[ZEPPELIN-760] Dropped redundent Flink Test
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/794
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-202599734
After reviewing the other InterpreterTest files, I find that none utilize a test where the output '.contains' a specific string. That specific test in Ignite is supposed to be testing for weather the interpreter is 'live' and can handle some basic commands which is verified by the assertion in the test. While it _should_ compile and regularly does. It seems to be consistently failing on one specific configuration, which leads me to believe the failure has more to do with the configuration than anything else.
Personally I would recommend dropping the line:
`assertTrue(result.message().contains(sizeVal + ": Int = " + ignite.cluster().nodes().size()));`
which is the line of failure in the Hadoop 1.6.0 builds, e.g. the hangup. The line preceding the proposed drop line asserts that the line is successfully executed. E.g. `val " + sizeVal + " = ignite.cluster().nodes().size()` does successfully execute. The test that is causing failure is that what ever is returned by that line is not what one would expect. Again, ignite is the only interpreter to run this sort of test.
The ability to handle companion objects for all Ignite users seems more important than a possibly unexpected number of nodes reported in a very specific build of Hadoop.
Just my .02.
I have no problem resetting the Ignite files.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-202538580
I moved the test to the end on a hunch that max fix the build issue. @Leemoonsoo can you run again on your machine? I think the code is solid- it just seems to be crashing on random spots in the CI build. When I build locally it crashes (consistently) in the Spark interpreter (even on fresh clone).
If this doesn't work- I'll just drop the Ignite support. Hate to do it bc I think the solution is sound its just bad tests :/
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-204069497
merging if no more comment
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/794
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201109499
once again failing on npm install.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201134262
@rawkintrevo Thanks for the contribution.
[One of test profile of last CI run](https://travis-ci.org/apache/incubator-zeppelin/jobs/118366944) fails with following error. Could you take a look?
```
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ zeppelin-scalding ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 1 source file to /home/travis/build/apache/incubator-zeppelin/scalding/target/test-classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /home/travis/build/apache/incubator-zeppelin/scalding/src/test/java/org/apache/zeppelin/scalding/ScaldingInterpreterTest.java:[132,15] method testNextLineInvocation() is already defined in class org.apache.zeppelin.scalding.ScaldingInterpreterTest
[INFO] 1 error
[INFO] -------------------------------------------------------------
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: [ZEPPELIN-760] Companion object b...
Posted by rawkintrevo <gi...@git.apache.org>.
Github user rawkintrevo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/794#issuecomment-201485432
Failed at:
Zeppelin: Server and Display system apis this time? Run it again?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---