R Interpreter for Zeppelin

This is the initial PR for an R Interpreter for Zeppelin.  There's still some work to be done (e.g., tests), but its useable, it brings to Zeppelin features from R like its library of statistics and machine learning packages, as well as advanced interactive visualizations.  So I'd like to open it up for others to comment and/or become involved.
Summary:

There are two interpreters, one emulates a REPL, the other uses knitr to weave markdown and formatted R output.  The two interpreters share a single execution environment.

Visualisations:  Besides R's own graphics, this also supports interactive visualizations with googleVis and rCharts.  I am working on htmlwidgets (almost done) with the author of that package, and a next-step project is to get Shiny/ggvis working.  Sometimes, a visualization won't load until the page is reloaded.  I'm not sure why this is.

Licensing:  To talk to R, this integrates code forked from rScala.  rScala was released with a BSD-license option, and the author's permission was obtained.

Spark:  Getting R to share a single spark context with the Spark interpreter group is going to be a project.  For right now, the R interpreters live in their own "r" interpreter group, and new spark contexts are created on startup.

Zeppelin Context:  Not yet integrated, in significant part because there's no ZeppelinContext to talk to until it lives in the Spark interpreter group.

Documentation:  A notebook is included that demonstrates what the interpreter does and how to use it.

Tests:  Working on it...

P.S.:  This is my first PR on a project of this size; let me know what I messed up and I'll try to fix it ASAP.

The check is failing because R is missing in Travis-this is ready for people to start playing with.

Ok I'm temporarily giving up on Travis - these test failures seem to all be spurious.

All:  This should now be functional.  Tests pass (there may still be some spurious errors on Travis, but I'm not seeing them).  And it should run out-of-box.  People should definitely start looking at it.

On the basic RInterpreter notebook, the first paragraph fails with:

could not find function "repr"

The following paragraph hits "knit" and fails with
org.apache.zeppelin.rinterpreter.rscala.RException
at the .sparkREnv directive.
What assumptions/prerequisites is it looking for in the installation?

If you look in the log, it should show what packages are missing. Repr shouldn't be needed unless something else is going wrong.
Can you send me your complete log?
It sounds like what's happening is that initialization is failing if some packages are missing and that definitely has to be fixed.

On Sep 22, 2015, at 6:47 PM, Jeff A. Bowles<EMAIL_ADDRESS>wrote:
On the basic RInterpreter notebook, the first paragraph fails with:
could not find function "repr"
The following paragraph hits "knit" and fails with
org.apache.zeppelin.rinterpreter.rscala.RException
at the .sparkREnv directive.
What assumptions/prerequisites is it looking for in the installation?
—
Reply to this email directly or view it on GitHub.

@jabowles Pls give what I just pushed a try and see if it resolves your issue.  I really need a log to be sure I've gotten to the bottom of it, but this may do it.

@elbamos found it working great in all scenarios involving SparkR, KnitR and various plots/charts. Great work !!!

@elbamos
Thanks for the contribution and
I briefly reviewed your codes and here's few comment.
1. Scala->R invocation
You seem to use rscala and separate connection to make scala -> r invocation.
I think SparkR already have RBackend that support r -> scala invocation and the same way we do in PySparkInterpreter, this one way connection can be also used to scala -> r without introducing much complexes and additional socket connection.
Using similar technique that PySparkInterpreter does for py4j in R Interpreter will help

Much smaller code base while there are no need for additional socket connection beside the SparkR provided one.
More consistent way with already existing PySparkInterpreter

2. KnitR Inteprreter
Seems like calling knit2html() is the role.
In this case, i think it more make sense to make a function and inject into RInterpreter (like z.show() in SparkInterpreter), rather than making separate interpreter.
3. Author, Copyrights and Maintainer tag
Zeppelin discourages Author tag. as well as Copyright / Maintainer if it is not really necessary.
Your contribution history will kept in commit history and archived in mailing list.

@Leemoonsoo
Please let me know who from the PPMC I can work with to resolve the continuing problems with the travis build and zeppelin-spark architecture.
Regarding your comments, as I've explained in our e-mail exchange:
1.  Scala-R invocation
I am not using rscala.
It is not possible to use the SparkR connection in the way you describe. I did look into this early on. There are numerous packages for interfacing the jvm and R.  None of them use two-way connections.
The python-spark and zeppelin integrations you describe leverage an external dependency.  There is no comparable package available for R that has a compatible license.
2. Knitr*
As I understand, you don't use R, so it may seem strange to have a separate interpreter rather than a function.  That's understandable.
The distinction between the r-repl and knitr interpreters makes perfect sense for people who are coming from R.  The repl and knitr handle code, and errors, and output, in fundamentally different ways.
They have different capabilities.  It is not possible, consistent with the zeppelin architecture, to put both capabilities into a single interpreter without making the use of that interpreter very unintuitive for someone coming from an R background.
The knit2html() command is something no R user would ever use when making use of R. It is perhaps best thought of as part of the "R operating system."
3, The author tag
That's really fine, but in my view this is the lowest-priority possible item.
The highest priority is the travis build problems.  Travis consistently fails building parts other than rzeppelin.
The other high priority is consistency with the Zeppelin-Spark interface, which has grown quite complex and difficult to use.
My users have had a long stream of issues getting Spark to work through zeppelin.  They get reported to me as rzeppelin issues, but have all turned out to be issues in the way zeppelin and spark interface, e.g., with conflicts between SPARK_HOME and spark.home.  rzeppelin needs to be consistent with the rest of the Zeppelin architecture in that regard.  This is not something I can fix because I don't own that code.

I am not using rscala.
It is not possible to use the SparkR connection in the way you describe. I did look into this early on. There are numerous packages for interfacing the jvm and R. None of them use two-way connections.
The python-spark and zeppelin integrations you describe leverage an external dependency. There is no comparable package available for R that has a compatible license.

Correction rscala -> forked-rscala.
You don't need to find any other external package for two-way connection. You can make two-way R<->JVM invocation without any external dependency with the same technique used in PySparkInterpreter.
PySpark implements one way connection Python->JVM similar to SparkR.
PysparkInterpreter leverage this one way connection and successfully made JVM->Python invocation without any external dependency.
I guess RInterpreter can do the same, with SparkR. I'll able to make JVM->R invocation without any external dependency and socket connection.
That'll simplify the code base. More precisely, more than 1000 lines of code.

As I understand, you don't use R, so it may seem strange to have a separate interpreter rather than a function. That's understandable.
The distinction between the r-repl and knitr interpreters makes perfect sense for people who are coming from R. The repl and knitr handle code, and errors, and output, in fundamentally different ways.
They have different capabilities. It is not possible, consistent with the zeppelin architecture, to put both capabilities into a single interpreter without making the use of that interpreter very unintuitive for someone coming from an R background.
The knit2html() command is something no R user would ever use when making use of R. It is perhaps best thought of as part of the "R operating system."

Yes i'm not familiar with R. So please convince me. What KnitR Interpreter doing is basically
     rContext.set(".zeppknitrinput", st.split("\n"))
     rContext.eval(".knitout <- knit2html(text=.zeppknitrinput, envir = rzeppelin:::.zeppenv)")
     rContext.getS0(".knitout")

and basic usage i found from KnitR website is
library(knitr)
?knit
knit(input)

So, to me, it's hard to imagine why functions like z.knite(input) does not make sense.
If you have use cases, please share.
By the way, KnitR is GPL license. I don't think Zeppelin can have a feature that depends on GPL licensed code.

That's really fine, but in my view this is the lowest-priority possible item.

License and Copyright problems are one of the hight priority item in Zeppelin project

The highest priority is the travis build problems. Travis consistently fails building parts other than rzeppelin.

Latest exception from your CI Build is
Caused by: java.lang.OutOfMemoryError: PermGen space
	at org.apache.zeppelin.rinterpreter.RSparkTest$$anonfun$3.apply$mcV$sp(RSparkTest.scala:51)
An exception or error caused a run to abort. This may have been caused by a problematic custom reporter.
Exception in thread "ScalaTest-main"
Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "ScalaTest-main"

Please try to increase PermGem memory option in the test.

My users have had a long stream of issues getting Spark to work through zeppelin. They get reported to me as rzeppelin issues, but have all turned out to be issues in the way zeppelin and spark interface, e.g., with conflicts between SPARK_HOME and spark.home. rzeppelin needs to be consistent with the rest of the Zeppelin architecture in that regard. This is not something I can fix because I don't own that code.

And https://issues.apache.org/jira/browse/ZEPPELIN-421 will address removal of spark.home property, in Zeppelin setting window. But until that, you can simply not trying to set spark.home.
And technically you don't own Zeppelin code but ASF does. but nothing stops you fix the problem.

Another one, is location of this package.
If it is implementing general R Interpreter, i think having separate submodule, like as-is make sense.
But if it is being in the same 'spark' interpreter group and implementing SparkR interpreter, that shares resources (eg. SparkContext) with SparkInterpreter, i think it more make sense to move into 'spark' submodule instead of having separate 'R' submodule.
Also, this approach is more consistent with package location of PySparkInterpreter, which is also part of Spark Interpreter group.

@leemoonsoo
I will be rebasing over the next few days.
I the meantime I'm going to go through the issues you raised in sequence...
1. The R-Scala Interface
I've explained several times that the proposal to use SparkR bi-directionally doesn't work.   I don't feel that I have more to add about that.
I will try to reduce the size of the code that originated in rscala.
If this is not going to be acceptable to the PCCM, please tell me now.
2. KnitR
What you're proposing is that users enter the same boilerplate, which they would have to figure out for themselves, every time they want to use knitr.
Knitr and the repl are fundamentally two different ways for users to interact with R.  They have very different behaviors in terms of error reporting and handling visualizations.
If you don't want to trust me about this, then I suggest we ask some other R users what makes the most sense.
If this is not going to be acceptable to the PCCM, please tell me now.
3. KnitR GPL License
By the way, KnitR is GPL license. I don't think Zeppelin can have a feature that depends on GPL licensed code.
KnitR is an optional external dependency.  This is not a licensing problem.
It is also not a licensing problem to interact with GPL code that isn't supplied with Zeppelin.
For example, R itself is GPL code.  So it Zeppelin cannot interact with external GPL code, then there cannot be an R interpreter in Zeppelin at all.
Considering that Spark interacts with R, I think this issue is closed.
4. License any copyright
License and Copyright problems are one of the highest priority item in Zeppelin project
Huh?  What we were talking about is who gets identified in code as the author.  That is obviously not a license/copyright issue, its an issue of credit.
You said that it is discouraged to identify anybody as the author in Apache projects.
However, the current code does identify authors, with you identified as the principal author.
So, at this point I'm not sure what you're referring to?
5. Location of the Package
Its fine with me if it goes under /spark.  The reason its in /r is that it simplifies testing and development.  Someone will have to merge the two build scripts; I'm using scalatest for testing.
6. Travis Builds
Actually the error in the travis logs begins with this:

15/09/23 03:12:22 INFO HiveMetaStore: No user is added in admin role, since config is empty
15/09/23 03:12:24 WARN SparkInterpreter: Can't create HiveContext. Fallback to SQLContext
java.lang.reflect.InvocationTargetException
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
at > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45> )
at java.lang.reflect.Constructor.newInstance(Constructor.java:526)

That's not coming from rzeppelin.  That's coming from the SparkInterpreter when rzeppelin asks it to initiate a spark backend.
This is what I mean about issues in the spark-zeppelin interface.

@Leemoonsoo @sourav-mazumder @emaasit @felixcheung
Actually, I just watched the youtube video in which, in September, Lee and Felix demo'd this code at the Seattle R users group.
Lee, I will be e-mailing you my phone number.  Call me.

@elbamos Hi, You're misunderstanding the authorship of codes from Apache project including Zeppelin.  You should remove your developer tag in the r/pom.xml and delete the files among r/R/rzeppelin/LICENSE. If you want to describe some instructions like r/R/rzeppelin/DESCRIPTION and r/R/rzeppelin/README, please use our documentation rules. You should also delete r/R/rzeppelin/NEWS for the same reason. And it's a virtue of not re-indenting any codes you are not editing even though it's wrong. And you divide the supporting of spark 1.4.2 and the being changeable the version of scala to separate issues. This also be a virtue that one issue resolves only one problem.  Even if your codes work perfectly but your contribution ignores general ways. I'm a expert of R, thus I don't know how your codes are good but you should consider several things before you contribute to Zeppelin and another Apache project.
@Leemoonsoo I think that we need to enrich our documentations. The title of this PR doesn't have any connected number of issue from JIRA.

@jongyoul
I have no idea what you were trying to say.
It appears to me that your primary concern is the names identified in the pom files as the authors?  Is that correct?
(Many of the files you are referencing, under rzeppelin/, are part of the source that has to be there for an R package.)
It appears to me that I have contributed in the correct manner, but there may be some people affiliated with this project who have other issues.

@elbamos Could you please tell me the usage of r/src/main/scala/scala/Console.scala? I can't find the codes calls Console. And If you want to include some files not to check in apache-rat, you should tell us the reason. This is even a source file and will be included by binary.

@elbamos Do you mean that it's of to located the files under the rzeppelin/, and do you mean rzeppelin is not a part of the contribution?

@jongyoul What's excluded from rat are files that get generated automatically during the build process -- they are the R equivalent of a .class or .jar file.
There are two rzeppelin folders.  One contains R source code.  The other contains the actual compiled R package.
Console.scala is used to create a virtual interface to R.  I believe its called by an implicit, but that is actually part from rscala that I haven't tinkered with.

@elbamos Do you include the kind of R source codes? If you do, Do you have a plan to to follow up the R sources if R is updated?

@jongyoul
I apologize for this, but I find it almost impossible to follow what you are trying to say.
The R source code is -- obviously -- included.

@elbamos that was not your code in Sept. That was 100% my own. If you watch closely, check the branch or check the notebook that I have on github, you would notice how the graphing is done is via a helper function, which would be completely different from your approach on this PR.
I'm going to what you post on my repo. If you want to continue you want to complain please feel free to on a different channel.

@felixcheung You're clearly lying since your code never worked and still doesn't. The code is right there on your git -- or rather, it isn't there since you never did the work.  I kept all of our e-mail exchanges in case I needed the evidence.  This discussion belongs in your repo since you are the wrongdoer.  That is where it will stay.

@elbamos just because you said it wouldn't, didn't mean my implementation wouldn't work. I have another user verified that it worked, and then I encouraged him to test your PR here.
This is the last I'm going to add on this topic. I have moved on and you should too.

@felixcheung You're lying.
In fact I found out about what you'd done from a user who contacted you after seeing your slides, and tried to use your git but of course there isn't anything there because you didn't do any work.
This will not go away -- it is going to follow you for a long time.

Sometimes people can argue then I see the passion.
But when people don't respect each other, it easily become blaming and fighting.
Let's try to not have blaming and fighting, but productive argue.
@elbamos
1. The R-Scala Interface

I've explained several times that the proposal to use SparkR bi-directionally doesn't work. I don't feel that I have more to add about that.
I will try to reduce the size of the code that originated in rscala.
If this is not going to be acceptable to the PCCM, please tell me now.

I described multiple times about PySparkInterpreter made successful bi-directional invocation with one way connection provided by py4j, and the some idea can applied to SparkR.
It's up to you taking the idea or not. But i just want to mention that:

Consider consistent way of communicating from Zeppelin to PySparkInterpreter. That'll later help new contributor understand both PySParkInterpreter, RInterpreter in the same way, together. So can expect more contributors.
Less chances of problem, it only requires single socket connection between R and JVM.
Code base will be reduced. Easier maintenance as well as faster code review.

2. KnitR

What you're proposing is that users enter the same boilerplate, which they would have to figure out for themselves, every time they want to use knitr.
Knitr and the repl are fundamentally two different ways for users to interact with R. They have very different behaviors in terms of error reporting and handling visualizations.
If you don't want to trust me about this, then I suggest we ask some other R users what makes the most sense.
If this is not going to be acceptable to the PCCM, please tell me now.

I explained why i'm thinking it can be implemented as a function z.knitr(input).
You said you're R user. So i'm asking you, can you please provide some use cases or examples why KnitR need to implemented as a separate Interpreter, not a function? You can show how different KnitR and Repl in terms of

ways for users to interact
error reporting
handling visualizations

3. KnitR GPL License

KnitR is an optional external dependency. This is not a licensing problem.
It is also not a licensing problem to interact with GPL code that isn't supplied with Zeppelin.
For example, R itself is GPL code. So it Zeppelin cannot interact with external GPL code, then there cannot be an R interpreter in Zeppelin at all.
Considering that Spark interacts with R, I think this issue is closed.

I'm not sure how Spark handles license issue with R. My guess is, generally GPL licensed compiler generated output is not applied GPL license. So it could be okay to SparkR give input to R and get output from it and use.
KnitR is i think little bit different. It's running as a library. It is dynamically linked with Zeppelin through R.
Zeppelin - R - KnitR

If you think it's okay, that means not only KnitR but any other GPL licensed library can used inside of Zeppelin in the same way.
4. License any copyright

License and Copyright problems are one of the highest priority item in Zeppelin project
Huh? What we were talking about is who gets identified in code as the author. That is obviously not a license/copyright issue, its an issue of credit.
You said that it is discouraged to identify anybody as the author in Apache projects.
However, the current code does identify authors, with you identified as the principal author.
So, at this point I'm not sure what you're referring to?

The current code with Author tag (at least my name in Author tag) is written before the copyright is transferred to ASF from NFLabs, by signing https://www.apache.org/licenses/software-grant.txt.
And thanks for pointing me that there're author tags remains, i'm going to remove that.
So please consider your one, too.
6. Travis Builds

Actually the error in the travis logs begins with this:
15/09/23 03:12:22 INFO HiveMetaStore: No user is added in admin role, since config is empty
15/09/23 03:12:24 WARN SparkInterpreter: Can't create HiveContext. Fallback to SQLContext
java.lang.reflect.InvocationTargetException
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:57)
at > > sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45> > )
at java.lang.reflect.Constructor.newInstance(Constructor.java:526)
That's not coming from rzeppelin. That's coming from the SparkInterpreter when rzeppelin asks it to initiate a spark backend.
This is what I mean about issues in the spark-zeppelin interface.

You know what java.lang.OutOfMemoryError: PermGen space means.
Then you'll able to fix it no matter where it is coming from. The first thing you can try is increasing PermGem space for the testing.

And if you are not comfortable working with Zeppelin community, not because of you're wrong or Zeppelin community is wrong, but maybe just because of the way we want to go and how we want to get there is little bit different. Keeping RInterpreter as a 3rd party interpreter is another option.
I think in the future, Zeppelin will have more intuitive way to load and run 3rd party Interpreters (although it's already possible without rebuild Zeppelin in a simple way. ie. by deploying binaries under /interpreter and adding configuration into zeppelin-site.xml). Then using 3rd party interpreter will be much more easier. Maybe Just few clicks to download and use it.
So keeping as a 3rd party interpreter and take more control of it is another way you can choose.
I personally think having SparkR as built-in interpreter is really nice to have for users.
But having rich 3rd party interpreter community is also important for Zeppelin. (And that's why Zeppelin community need more intuitive way to use 3rd party interpreter).
So both way, I'm feeling totally fine. Choice is yours.

To anyone who may be coming here from today's blog post and confused by these comments about the current status:
The RInterpreter for zeppelin works fine with Spark 1.4.x, and should compile straight out of the box.
The travis build is failing because of bugs in other parts of the way Zeppelin interfaces with Spark.
I expect to be pushing a new revision of the Interpreter, with Spark 1.5.x support, in the next day or so.
Why the comments about the PR?  Yesterday I discovered that someone (Felix Cheung) had been demo'ing this code to R user groups on the West Coast, claiming to have done the work. One of the lead Zeppelin developers was present for at least one of these demos.
Short version of the backstory:  When I'd announced the project to the Zeppelin developer's list, Felix had pleaded to participate. I said "ok," but Felix failed to do any work.  Not a single line of code.  When I told him I was making the PR without him, he threatened that if he was not given credit he would sabotage the project by claiming that the code had been stolen from him.  He carried-through with his threat, and I had to produce chat logs and e-mail chains to demonstrate that he was lying.  Unfortunately, he's apparently also friends with at least one of the lead Zeppelin developers.
In any event, I will be deciding after the next push how I will move forward.

@elbamos Please do not blame people in your PR. That's out of your PR's scope.
And I thought you and Felix both agreed to move forward, If you think that's really a problem in your code contribution, then please come back later after the problem is solved clearly. Just blaming people here really does not help you, in anyway.

The current push should resolve some issues with changes in the Spark-Zeppelin interface that had created issues for users, as well as support for 1.5.1.
Knitr support is improved, and the reason for a separate knitr interpreter may be clearer now.
The amount of code borrowed from rscala is reduced.
I did not address issues with @author tags, or files under the R/ folder.  The reason is, to be blunt, I don't understand what the precise concerns actually are.
Please note that I changed .travis.yml to only use spark 1.4 and later.  I'm sure there is a better way to do it, but I'm just not in a position to try to figure out the entire Zeppelin build process.
Integrating this with the rest of zeppelin is going to take some work regarding pom's, travis, and so forth.  I can do a lot of that, but I'm going to need to discuss it with the people who have been "owning" those files.  There are just too many moving pieces here.
Because of the size of this (which is, unfortunately, necessary), posting here is probably not the most efficient way. That is also true because certain people chose to use this PR as a forum to air other issues.  Therefore, it would be better if reviewers e-mail me directly.

The current version of this is here:  https://github.com/elbamos/Zeppelin-With-R

Could you try to create Travis job for your fork repository to test the changes?
Shall this PR be closed in favor on new PR from the new branch?

@hsaputra There shouldn't be any changes in the new branch that are relevant to Travis CI testing.  If there's anything I can do to move the Travis issue forward -- I'm totally open to advice.
Have you tried the new branch, by the way?

I have sent a mail to the dev list http://mail-archives.apache.org/mod_mbox/incubator-zeppelin-dev/201512.mbox/<5683E471.9010001%40apache.org>
Happy to further discuss.

I've made some progress on testing CI,
at branch
https://github.com/Leemoonsoo/incubator-zeppelin/tree/rinterpreter_jan
at that passes one testing profile (spark-1.6) on CI, but fails on the other profiles (spark-1.5, spark-1.4 ...)
https://travis-ci.org/Leemoonsoo/incubator-zeppelin/builds/103512174
Any people familiar with following exception?
16/01/20 02:52:10 INFO SparkContext: Running Spark version 1.5.2
16/01/20 02:52:10 INFO SecurityManager: Changing view acls to: travis
16/01/20 02:52:10 INFO SecurityManager: Changing modify acls to: travis
16/01/20 02:52:10 INFO SecurityManager: SecurityManager: authentication disabled; ui acls disabled; users with view permissions: Set(travis); users with modify permissions: Set(travis)
16/01/20 02:52:11 INFO Slf4jLogger: Slf4jLogger started
16/01/20 02:52:11 INFO Remoting: Starting remoting
16/01/20 02:52:11 INFO Remoting: Remoting started; listening on addresses :[akka.tcp://sparkDriver@<IP_ADDRESS>:47557]
16/01/20 02:52:11 INFO Utils: Successfully started service 'sparkDriver' on port 47557.
16/01/20 02:52:11 INFO SparkEnv: Registering MapOutputTracker
16/01/20 02:52:11 INFO SparkEnv: Registering BlockManagerMaster
16/01/20 02:52:11 INFO DiskBlockManager: Created local directory at /tmp/blockmgr-d5f6db46-c354-457f-ac04-514cb750f5f7
16/01/20 02:52:11 INFO MemoryStore: MemoryStore started with capacity 530.3 MB
16/01/20 02:52:11 INFO HttpFileServer: HTTP File server directory is /tmp/spark-70746e88-64b6-4e5f-a89a-0acc469f220c/httpd-ee337ade-6d23-4fcb-ac42-1e649e11c626
16/01/20 02:52:11 INFO HttpServer: Starting HTTP Server
16/01/20 02:52:11 INFO Utils: Successfully started service 'HTTP file server' on port 42899.
16/01/20 02:52:11 INFO SparkEnv: Registering OutputCommitCoordinator
16/01/20 02:52:11 INFO Utils: Successfully started service 'SparkUI' on port 4040.
16/01/20 02:52:11 INFO SparkUI: Started SparkUI at http://<IP_ADDRESS>:4040
16/01/20 02:52:11 INFO SparkContext: Added JAR file:/home/travis/build/Leemoonsoo/incubator-zeppelin/interpreter/spark/zeppelin-spark-0.6.0-incubating-SNAPSHOT.jar at http://<IP_ADDRESS>:42899/jars/zeppelin-spark-0.6.0-incubating-SNAPSHOT.jar with timestamp<PHONE_NUMBER>807
16/01/20 02:52:11 INFO FairSchedulableBuilder: Created default pool default, schedulingMode: FIFO, minShare: 0, weight: 1
16/01/20 02:52:11 WARN MetricsSystem: Using default name DAGScheduler for source because spark.app.id is not set.
16/01/20 02:52:11 INFO AppClient$ClientEndpoint: Connecting to master spark://testing-gce-b9b06e11-646b-46d3-9860-831cc900b4f7.c.eco-emissary-99515.internal:7071...
16/01/20 02:52:31 ERROR SparkUncaughtExceptionHandler: Uncaught exception in thread Thread[appclient-registration-retry-thread,5,main]
java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.FutureTask@12241de8 rejected from java.util.concurrent.ThreadPoolExecutor@12773a6[Running, pool size = 1, active threads = 0, queued tasks = 0, completed tasks = 1]
	at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2048)
	at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:821)
	at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1372)
	at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:110)
	at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anonfun$tryRegisterAllMasters$1.apply(AppClient.scala:96)
	at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anonfun$tryRegisterAllMasters$1.apply(AppClient.scala:95)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
	at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:108)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.mutable.ArrayOps$ofRef.map(ArrayOps.scala:108)
	at org.apache.spark.deploy.client.AppClient$ClientEndpoint.tryRegisterAllMasters(AppClient.scala:95)
	at org.apache.spark.deploy.client.AppClient$ClientEndpoint.org$apache$spark$deploy$client$AppClient$ClientEndpoint$$registerWithMaster(AppClient.scala:121)
	at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anon$2$$anonfun$run$1.apply$mcV$sp(AppClient.scala:132)
	at org.apache.spark.util.Utils$.tryOrExit(Utils.scala:1119)
	at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anon$2.run(AppClient.scala:124)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:304)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
	at java.lang.Thread.run(Thread.java:745)

Hi Moon,
From the exception looks like it is trying to test Spark in a standalone
mode and it is not able to register the master process.
Ideally the standalone cluster (with its master and slave) should be
created before hand. I tested that way and it always worked that way.
I'm assuming this is output from a testing done by CI/automated testing.
Most probably there may be a script missing which is supposed to start the
Spark standalone cluster before hand.
Hope this helps.
Regards,
Sourav
On Wed, Jan 20, 2016 at 5:33 PM, Lee moon soo<EMAIL_ADDRESS>wrote:

I've made some progress on testing CI,
at branch
https://github.com/Leemoonsoo/incubator-zeppelin/tree/rinterpreter_jan
at that passes one testing profile (spark-1.6) on CI, but fails on the
other profiles (spark-1.5, spark-1.4 ...)
https://travis-ci.org/Leemoonsoo/incubator-zeppelin/builds/103512174
Any people familiar with following exception?
16/01/20 02:52:10 INFO SparkContext: Running Spark version 1.5.2
16/01/20 02:52:10 INFO SecurityManager: Changing view acls to: travis
16/01/20 02:52:10 INFO SecurityManager: Changing modify acls to: travis
16/01/20 02:52:10 INFO SecurityManager: SecurityManager: authentication disabled; ui acls disabled; users with view permissions: Set(travis); users with modify permissions: Set(travis)
16/01/20 02:52:11 INFO Slf4jLogger: Slf4jLogger started
16/01/20 02:52:11 INFO Remoting: Starting remoting
16/01/20 02:52:11 INFO Remoting: Remoting started; listening on addresses :[akka.tcp://sparkDriver@<IP_ADDRESS>:47557]
16/01/20 02:52:11 INFO Utils: Successfully started service 'sparkDriver' on port 47557.
16/01/20 02:52:11 INFO SparkEnv: Registering MapOutputTracker
16/01/20 02:52:11 INFO SparkEnv: Registering BlockManagerMaster
16/01/20 02:52:11 INFO DiskBlockManager: Created local directory at /tmp/blockmgr-d5f6db46-c354-457f-ac04-514cb750f5f7
16/01/20 02:52:11 INFO MemoryStore: MemoryStore started with capacity 530.3 MB
16/01/20 02:52:11 INFO HttpFileServer: HTTP File server directory is /tmp/spark-70746e88-64b6-4e5f-a89a-0acc469f220c/httpd-ee337ade-6d23-4fcb-ac42-1e649e11c626
16/01/20 02:52:11 INFO HttpServer: Starting HTTP Server
16/01/20 02:52:11 INFO Utils: Successfully started service 'HTTP file server' on port 42899.
16/01/20 02:52:11 INFO SparkEnv: Registering OutputCommitCoordinator
16/01/20 02:52:11 INFO Utils: Successfully started service 'SparkUI' on port 4040.
16/01/20 02:52:11 INFO SparkUI: Started SparkUI at http://<IP_ADDRESS>:4040
16/01/20 02:52:11 INFO SparkContext: Added JAR file:/home/travis/build/Leemoonsoo/incubator-zeppelin/interpreter/spark/zeppelin-spark-0.6.0-incubating-SNAPSHOT.jar at http://<IP_ADDRESS>:42899/jars/zeppelin-spark-0.6.0-incubating-SNAPSHOT.jar with timestamp<PHONE_NUMBER>807
16/01/20 02:52:11 INFO FairSchedulableBuilder: Created default pool default, schedulingMode: FIFO, minShare: 0, weight: 1
16/01/20 02:52:11 WARN MetricsSystem: Using default name DAGScheduler for source because spark.app.id is not set.
16/01/20 02:52:11 INFO AppClient$ClientEndpoint: Connecting to master spark://testing-gce-b9b06e11-646b-46d3-9860-831cc900b4f7.c.eco-emissary-99515.internal:7071...
16/01/20 02:52:31 ERROR SparkUncaughtExceptionHandler: Uncaught exception in thread Thread[appclient-registration-retry-thread,5,main]
java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.FutureTask@12241de8 rejected from java.util.concurrent.ThreadPoolExecutor@12773a6[Running, pool size = 1, active threads = 0, queued tasks = 0, completed tasks = 1]
at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2048)
at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:821)
at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1372)
at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:110)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anonfun$tryRegisterAllMasters$1.apply(AppClient.scala:96)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anonfun$tryRegisterAllMasters$1.apply(AppClient.scala:95)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:108)
at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
at scala.collection.mutable.ArrayOps$ofRef.map(ArrayOps.scala:108)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint.tryRegisterAllMasters(AppClient.scala:95)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint.org$apache$spark$deploy$client$AppClient$ClientEndpoint$$registerWithMaster(AppClient.scala:121)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anon$2$$anonfun$run$1.apply$mcV$sp(AppClient.scala:132)
at org.apache.spark.util.Utils$.tryOrExit(Utils.scala:1119)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anon$2.run(AppClient.scala:124)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:304)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:745)
—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-173423103
.

Sounds right, except the existing PR should fail on spark 1.6 anyway. The version currently on my repo as 0.5.6 is up to date with 0.5.6 release.

On Jan 20, 2016, at 9:13 PM, sourav-mazumder<EMAIL_ADDRESS>wrote:
Hi Moon,
From the exception looks like it is trying to test Spark in a standalone
mode and it is not able to register the master process.
Ideally the standalone cluster (with its master and slave) should be
created before hand. I tested that way and it always worked that way.
I'm assuming this is output from a testing done by CI/automated testing.
Most probably there may be a script missing which is supposed to start the
Spark standalone cluster before hand.
Hope this helps.
Regards,
Sourav
On Wed, Jan 20, 2016 at 5:33 PM, Lee moon soo<EMAIL_ADDRESS>wrote:

I've made some progress on testing CI,
at branch
https://github.com/Leemoonsoo/incubator-zeppelin/tree/rinterpreter_jan
at that passes one testing profile (spark-1.6) on CI, but fails on the
other profiles (spark-1.5, spark-1.4 ...)
https://travis-ci.org/Leemoonsoo/incubator-zeppelin/builds/103512174
Any people familiar with following exception?
16/01/20 02:52:10 INFO SparkContext: Running Spark version 1.5.2
16/01/20 02:52:10 INFO SecurityManager: Changing view acls to: travis
16/01/20 02:52:10 INFO SecurityManager: Changing modify acls to: travis
16/01/20 02:52:10 INFO SecurityManager: SecurityManager: authentication disabled; ui acls disabled; users with view permissions: Set(travis); users with modify permissions: Set(travis)
16/01/20 02:52:11 INFO Slf4jLogger: Slf4jLogger started
16/01/20 02:52:11 INFO Remoting: Starting remoting
16/01/20 02:52:11 INFO Remoting: Remoting started; listening on addresses :[akka.tcp://sparkDriver@<IP_ADDRESS>:47557]
16/01/20 02:52:11 INFO Utils: Successfully started service 'sparkDriver' on port 47557.
16/01/20 02:52:11 INFO SparkEnv: Registering MapOutputTracker
16/01/20 02:52:11 INFO SparkEnv: Registering BlockManagerMaster
16/01/20 02:52:11 INFO DiskBlockManager: Created local directory at /tmp/blockmgr-d5f6db46-c354-457f-ac04-514cb750f5f7
16/01/20 02:52:11 INFO MemoryStore: MemoryStore started with capacity 530.3 MB
16/01/20 02:52:11 INFO HttpFileServer: HTTP File server directory is /tmp/spark-70746e88-64b6-4e5f-a89a-0acc469f220c/httpd-ee337ade-6d23-4fcb-ac42-1e649e11c626
16/01/20 02:52:11 INFO HttpServer: Starting HTTP Server
16/01/20 02:52:11 INFO Utils: Successfully started service 'HTTP file server' on port 42899.
16/01/20 02:52:11 INFO SparkEnv: Registering OutputCommitCoordinator
16/01/20 02:52:11 INFO Utils: Successfully started service 'SparkUI' on port 4040.
16/01/20 02:52:11 INFO SparkUI: Started SparkUI at http://<IP_ADDRESS>:4040
16/01/20 02:52:11 INFO SparkContext: Added JAR file:/home/travis/build/Leemoonsoo/incubator-zeppelin/interpreter/spark/zeppelin-spark-0.6.0-incubating-SNAPSHOT.jar at http://<IP_ADDRESS>:42899/jars/zeppelin-spark-0.6.0-incubating-SNAPSHOT.jar with timestamp<PHONE_NUMBER>807
16/01/20 02:52:11 INFO FairSchedulableBuilder: Created default pool default, schedulingMode: FIFO, minShare: 0, weight: 1
16/01/20 02:52:11 WARN MetricsSystem: Using default name DAGScheduler for source because spark.app.id is not set.
16/01/20 02:52:11 INFO AppClient$ClientEndpoint: Connecting to master spark://testing-gce-b9b06e11-646b-46d3-9860-831cc900b4f7.c.eco-emissary-99515.internal:7071...
16/01/20 02:52:31 ERROR SparkUncaughtExceptionHandler: Uncaught exception in thread Thread[appclient-registration-retry-thread,5,main]
java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.FutureTask@12241de8 rejected from java.util.concurrent.ThreadPoolExecutor@12773a6[Running, pool size = 1, active threads = 0, queued tasks = 0, completed tasks = 1]
at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2048)
at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:821)
at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1372)
at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:110)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anonfun$tryRegisterAllMasters$1.apply(AppClient.scala:96)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anonfun$tryRegisterAllMasters$1.apply(AppClient.scala:95)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:108)
at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
at scala.collection.mutable.ArrayOps$ofRef.map(ArrayOps.scala:108)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint.tryRegisterAllMasters(AppClient.scala:95)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint.org$apache$spark$deploy$client$AppClient$ClientEndpoint$$registerWithMaster(AppClient.scala:121)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anon$2$$anonfun$run$1.apply$mcV$sp(AppClient.scala:132)
at org.apache.spark.util.Utils$.tryOrExit(Utils.scala:1119)
at org.apache.spark.deploy.client.AppClient$ClientEndpoint$$anon$2.run(AppClient.scala:124)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:304)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:745)
—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-173423103
.

—
Reply to this email directly or view it on GitHub.

Guys, R interpreter is a big important and complicated contribution, hope we could make our last push to get that merged.
On my spare time I briefly went thought the this PR again and listed (in order of importance) a few things that I think we need to complete to speed things up and merge this one.
@elbamos @justsomeone1001 @sourav-mazumder @Leemoonsoo @felixcheung  please let me know what do you think about this plan.

[ ] fraction out changes to other modules out of this PR (will make things simpler to test and review)

spark-dependencies/pom.xml
spark/pom.xml
It should be possible to re-create them as a separate PRs if needed.

[ ] rebase on top of latest master

[ ] add\point to Docs
AFAIK for a user to enjoy R interpreter, there are some pre-requests like packages to be installed, etc. Is there any documentation on 'how to setup' the environment?
If we could have one under /docs/ - that would be awesome. It's very easy to add, but this would be very helpful to make more people use it and help us testing it. Here are docs on how to do that.

[ ] (alex) CI failing.

[ ] clarify minor copyright issue in r/R/rzeppelin

r/R/rzeppelin/LICENSE rises the question:
is r/R/rzeppelin/* a work under ASF or is that a third-party code?
in either case, from my understanding, it needs to be updated according to the links above (either by adding ASF headers to all the files and removing LICENSE, or by listing it in root project's NOTICE file)

[ ] fix /r/pom.xml formatting
[ ] make sure all other source code formatting is consistent
Like this, this and that, etc

If somebody could start with 1-3, I will be happy to help with 4 ASAP, hopefully will have more spare cycles by the end of the week.

I would be happy to help on point 3.
Regards,
Sourav
On Mon, Feb 8, 2016 at 3:36 PM, Alexander<EMAIL_ADDRESS>wrote:

Guys, R interpreter is a big important and complicated contribution, hope
we could make our last push to get that merged.
On my spare time I briefly went thought the this PR again and listed (in
order of importance) a few things that I think we need to complete to speed
things up and merge this one.
@elbamos https://github.com/elbamos @justsomeone1001
https://github.com/justsomeone1001 @sourav-mazumder
https://github.com/sourav-mazumder @Leemoonsoo
https://github.com/Leemoonsoo @felixcheung
https://github.com/felixcheung please let me know what do you think
about this plan.

fraction out changes to other modules out of this PR (will make things
simpler to test and review)

spark-dependencies/pom.xml

spark/pom.xml It should be possible to re-create them as a
separate PRs if needed.

rebase on top of latest master
3.
add\point to Docs
AFAIK for a user to enjoy R interpreter, there are some pre-requests
like packages to be installed, etc. Is there any documentation on 'how to
setup' the environment?
If we could have one under /docs/ - that would be awesome. It's very
easy to add, but this would be very helpful to make more people use it and
help us testing it. Here are docs
http://zeppelin.incubator.apache.org/docs/0.6.0-incubating-SNAPSHOT/development/howtocontributewebsite.html
on how to do that.
4.
(alex) CI failing.
5.
clarify minor copyright issue in r/R/rzeppelin
r/R/rzeppelin/LICENSE
https://github.com/apache/incubator-zeppelin/pull/208/files#diff-70a1eaac699ff7afe9f107a9aafe976dR2
rises the question:
is r/R/rzeppelin/* a work under ASF
http://www.apache.org/legal/src-headers.html#headers or is that a third-party
code http://www.apache.org/legal/src-headers.html#3party?
in either case, from my understanding, it needs to be updated
according to the links above (either by adding ASF headers to all the files
and removing LICENSE, or by listing it in root project's NOTICE file)
6.
fix /r/pom.xml formatting
7. make sure all other source code formatting is consistent Like this
https://github.com/apache/incubator-zeppelin/pull/208/files#diff-b1e5f7257eb27da9958f26d8c763d242R143,
this
https://github.com/apache/incubator-zeppelin/pull/208/files#diff-b1e5f7257eb27da9958f26d8c763d242R153
and that
https://github.com/apache/incubator-zeppelin/pull/208/files#diff-728dfe61ffb885af17a8b5c7efb44e75R37,
etc
If somebody could start with 1-3, I will be happy to help with 4 ASAP,
hopefully will have more spare cycles by the end of the week.
—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-181624753
.

@bzz Thank you for your attention and efforts to move this forward.  I am happy to do the work with respect to the issues you've identified (other than CI, where I'm simply stuck).
Regarding 2, 3, & 6  the version here https://github.com/elbamos/Zeppelin-With-R has documentation and is up-to-date against 0.5.6.
Regarding 1, there are some changes necessary to the substantive code under the spark/ package.  They are not major, but are necessary.  There are also some revisions to the build system in spark-dependencies and spark.  I don't think those are necessary, but they do resolve some spark-building issues that people email the list about.
Regarding 5 (licenses): the code under r/R is ASF.  Except that a good chunk of it is based on code that is BSD licensed (rscala).  (I.e., its a derivative work licensed under ASF but with portions licensed under BSD.)  I'm not sure how the license should appear in those cases?  Also, some of those files don't have licenses because they're automatically generated during the R-package build process; they're really the output of a build rather than the input.
I will need a day or two to turn to these -- I am also going to try not to respond further until the moratorium is up.

@bzz Thank you for your attention and efforts to move this forward!
I'm happy to do all the required work (but I'm really just stuck on CI).
I think you have hit the issues.   Let me give the status of a few:

(Other modules.)  There are small changes to spark/ that are necessary.  There are also changes to the pom.xml's in spark, spark-dependencies, and the main one.  I don't think they're necessary, but they do resolve some persistent issues people have had building with different variants of spark.

(Rebase.)  This:  https://github.com/elbamos/Zeppelin-With-R  Is up-to-date with 0.5.6.  I will bring this branch up to date with current master.

(Docs.)  Docs are included with the URL above and will be in the rebased PR.

(CI) Thank you!

(Licenses.)  The code under r/ is ASF, but some files are derived from BSD-licensed rscala code.  Also, some files under r/ are not code.  They're generated during the R-package build process, so they don't have licenses at the top.  I think this is some of what Jongyoul was on about.  I'm happy to take care of this however, but just need to know how it should look.

(pom.xml formatting.)  Can you clarify what's required?

(Source formatting.)  Will-do.

Also - we should probably change the build so that R is either a module, or builds only if R is present.  Something also needs to be done so it gets included in the binary build.
I will work on this over the next few days, but I'm trying to respect the moratorium.

@bzz sounds like a good plan.

Guys, thank you for the feedback!
To pursue our plan I have summarize it after the discussion and took the liberty of mentioning those who volunteered to help.

[ ] (amos) remove changes that are not required from this PR, in particular in other modules

spark-dependencies/pom.xml
spark/pom.xml

Create new PRs with those issues.

[ ] (amos) rebase all work on top of latest master

[ ] (amos, sourav) add\point to Docs
Move awesome docs from README of https://github.com/elbamos/Zeppelin-With-R to the /docs/. Here are instructions on how to do that.

[ ] (alex) fix CI failure.

[ ] clarify minor copyright issue in r/R/rzeppelin
All contributed files (generated as well) should have Apache header, LICENSE file removed, third-party under BSD license should not be changed, but mentioned in zeppelin-distribution/src/bin_license/LICENSE

[ ] fix /r/pom.xml formatting: i.e indention here and here

[ ] (amos) make sure all other source code formatting is consistent
Like this, this and that, etc

Further improvements (in next PR after this got merged):

update build scripts: either add R a module, or builds only if R is present.
include R interpreter in the binary build

better be taken care of in further PRs (will be glad to help).

@bzz  I'm working on this now and will push something later tonight.  Is there anything on CI that I should take into account?  Thanks.

Sounds great, I'm looking forward.
Right now I do not see anything specific that need to be taken into account.

Rebase done, but not fully tested...  Work ongoing...

I have reviewed and commented inline.
Generally it looks great, but I have the feeling that we could dramatically speed merging it up by reducing the scope of changes.
What do you think about creating a new, much simpler PR, with just copy of ./r/ from here (with ASF headers and fixed formatting) + adding ./docs and updated zeppelin-distribution/src/bin_license/LICENSE?
Otherwise we are at risk that changes in different modules might slow reivew\merge process, as well as struggling with git rebase artifacts is not the best way to spend the time (at least in my experience).

Concerning vendor-repo, it's already merged by #701, thus there's no need to be added separately.

@bzz All of the issues you identified are git-rebase artifacts.  None of them are intended as part of the PR.  Please keep in mind the PR was originally in August, so there are a lot of opportunities for artifacts like this to arise.
One of the remaining tasks I have, is to eliminate changes that aren't necessary to the PR.  Part of that is comparing against current master to make sure inadvertent reversions like these aren't taking place.  That's one of the items on your task list, and it isn't complete yet.
The intended changes to the PR are:

under r/
A few changes under spark/
Under docs/
A small number of changes to the spark-related dependencies in pom files

There are also some changes -- not many -- that are not related to the PR, but were necessary to get Zeppelin to compile at all.  For example, when I pulled recent changes last night, there were syntax errors in one of the javascript files.  I imagine that these are issues that relate to other PRs conflicting, or to errors in the rebase process. But they will show-up in this interface as changes in the PR.
@jongyoul Yes, you're correct.  That isn't intended as part of the PR.

@bzz @jongyoul This push should have removed the extraneous git artifacts.  It is still a lot of files.  If we want to split it up into smaller bits, I suggest that we make the documentation and the tests separate PRs.  That's a natural way to break it up.
Note: This is intended to address @bzz's issues 1-3.  It does not yet address issues 5-7  That's tomorrow.

Great progress @elbamos thank you! @jongyoul thank you for keeping an eye
Please excuse me, most of comments above are on issues 5-7 and may be preliminary, but hope we could nail them down as well.
We need 5 (clarify license\headers) before fixing the CI, as Apache license header check is one of the steps of the build and will fail the CI if not present.

@bzz I'm trying to implement these changes now.  A few comments:

The DESCRIPTION file -- this is a file with a particular format.  The license goes in the License line of the file, which is the case here.

The LICENSE file -- for R packages, this is where custom licenses go.  I've put the Apache license text into the file.

Files under r/R/*.R -- The files you identified here are BSD licensed.  They're from rscala, and no changes have been made.

RClient.scala -- The original file did not include the Apache license, but the file has been modified.   I've added a line to zeppelin-distribution/src/bin_license --- are we sure, though that that's correct?  We're not including binaries, we're making a derivative work from source.

Console.scala - The license is BSD and BSD-style.  The bulk of the file is part of the source to scala itself, which is licensed under a "BSD-style" license.  http://www.scala-lang.org/old/node/249.html  There are changes that were part of rscala, which is BSD.

Pushing momentarily...

I apologize if this is not the right forum, but I am confused as to why there are 2 PRs for R integration. Is someone wasting their time, or is it expected that both will be merged? I have read both and cannot figure this out.

I second @btiernay 's comment. I do greatly appreciate you guy's efforts here. Look forward to the final merge!

@btiernay @zhiboz thank you for feedback, if you could please express it on the dev@ mailing list that would be much apprecaited and would be the way reach broader community.

@elbamos got it, about BSD licence files. Let me double-check everything and get back to you with it by Monday

@bzz Hey just checking-in to see how we're doing and if there's anything I can do at this point to help move us along toward merge.

may be this URL can help...
http://jtleek.com/protocols/travis_bioc_devel/
:+1:

@jeffsteinmetz Thanks for your comment.  By default, r and knitr are in the spark interpreter group, so Zeppelin lets them be called by %r/%spark.r or %knitr/%spark.knitr as equivalents, respectively.  I've put in a note in the documentation to clarify.

I just pushed fixes for the issue spotted by @jeffsteinmetz, the extraneous R Studio file, and (I think) the misalligned function.  @hsaputra can you explain why you believe that code was copied verbatim from someplace it shouldn't have been?  "aelberg" is me.

Oh my goodness @elbamos , you have so many aliases =P
I apologize about the confusion.

Great progress, @elbamos thank for quick updates!  @hsaputra and @jeffsteinmetz thank you very much for your help!
I was able to reproduce same build failure as on CI, looking into it now.

Guys, please correct me if I'm wrong but current R interpreter implementation, in order to be compiled, requires R installed on user's machine AND it is included in the default build (at least right now) - that would result in build failing for every user of Zeppelin out there after pulling the change.
I think what we may do, without breaking Zeppelin build for all the users, is simply exclude R interpreter build from default build cycle of mvn package and put it under profile, i.e -P r as we do with scalding, pyspark and other interpreters \w external dependencies . This way those who do not need R support will not be affected, at the same time those who do - will be able to get it.
What do you guy think? Then I could update CI to use this profile as well.

As such that approach makes sense to me.
But wondering how will this work for the Zeppelin binary ? Will Zeppelin
binary not have the R support by default because of this ? So whoever wants
to create his own build ?
So instead of that approach I would suggest if this check/constraint can be
moved at run time level instead of compile time. So whoever tries to run
the interpreter gets the error and he/she then goes and install R. I
believe both SparkR and RStudio work in similar fashion.
Regards,
Sourav
On Wed, Feb 24, 2016 at 5:21 AM, Alexander<EMAIL_ADDRESS>wrote:

Guys, please correct me if I'm wrong but current R interpreter
implementation, in order to be compiled, requires R installed on user's
machine AND it is included in the default build (at least right now) - that
would result in build failing for every user of Zeppelin out there after
pulling the change.
I think what we may do, without breaking Zeppelin build for all the users,
is simply exclude R interpreter build from default build cycle of mvn
package and put it under profile, i.e -P r as we do with scalding,
pyspark and other interpreters \w external dependencies . This way those
who do not need R support will not be affected, at the same time those who
do - will be able to get it.
What do you guy think? Then I could update CI to use this profile as well.
—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-188252250
.

When zeppelin is built with the -Ppyspark profile it downloads the entire spark binary (as @bzz mentioned).  So a -P r makes sense.  During a zeppelin release, it could build zeppelin with an R profile in the full packaged build.  I believe -Ppyspark is in the profile for the full build upon version releases.  R could be as well.

Agree with @jeffsteinmetz and @bzz

I can also update the script I contributed for the virtual machine found in /scripts/vagrant/zeppelin-dev to include the required R components and commonly used R packages.  Since SparkR is a core component of Apache Spark, as is Python support native to Spark, I think having R as part of the virtual machine base build makes sense.

I agree with the proposal above. @jeffsteinmetz I had trouble finding the script you're referring to - can you provide a link?

@elbamos :  https://github.com/apache/incubator-zeppelin/tree/master/scripts/vagrant/zeppelin-dev

@sourav-mazumder this is not changing the final convenience binary for the release - as we will update release script too, to include this profile (+ docs for Release Manager on what's required for a build)

@elbamos - feel free to comment on this ticket:
https://issues.apache.org/jira/browse/ZEPPELIN-700
Can you provide any insight (via comments in ticket 700 as well if you wish) as to "why" each package is required.
i.e. why is the "evaluate" package required, and would the R interpreter work with out it?

@elbamos I tested a full build of a Zeppelin distribution using this PR in a virtual machine.
When running from the source directory (not the distribution directory) - this works:
%r
2+2

However when running from the distribution directory, I get the error listed below.
Virtual Machine additions include:
R version 3.2.3
packages:
- evaluate
- knitr
- repr
- htmltools
- base64enc
exact steps to replicate:
git clone https://github.com/apache/incubator-zeppelin.git
git fetch origin pull/208/head:208
git checkout 208
mvn clean package -Pbuild-distr -Pspark-1.6 -Phadoop-2.4 -Ppyspark -DskipTests
cd zeppelin-distribution/target/zeppelin-0.6.0-incubating-SNAPSHOT/zeppelin-0.6.0-incubating-SNAPSHOT
bin/zeppelin.sh

error (when using r or knitr interpreter):
org.apache.thrift.transport.TTransportException
	at org.apache.thrift.transport.TIOStreamTransport.read(TIOStreamTransport.java:132)
	at org.apache.thrift.transport.TTransport.readAll(TTransport.java:86)
	at org.apache.thrift.protocol.TBinaryProtocol.readAll(TBinaryProtocol.java:429)
	at org.apache.thrift.protocol.TBinaryProtocol.readI32(TBinaryProtocol.java:318)
	at org.apache.thrift.protocol.TBinaryProtocol.readMessageBegin(TBinaryProtocol.java:219)
	at org.apache.thrift.TServiceClient.receiveBase(TServiceClient.java:69)
	at org.apache.zeppelin.interpreter.thrift.RemoteInterpreterService$Client.recv_interpret(RemoteInterpreterService.java:237)
	at org.apache.zeppelin.interpreter.thrift.RemoteInterpreterService$Client.interpret(RemoteInterpreterService.java:222)
	at org.apache.zeppelin.interpreter.remote.RemoteInterpreter.interpret(RemoteInterpreter.java:222)
	at org.apache.zeppelin.interpreter.LazyOpenInterpreter.interpret(LazyOpenInterpreter.java:93)
	at org.apache.zeppelin.notebook.Paragraph.jobRun(Paragraph.java:213)
	at org.apache.zeppelin.scheduler.Job.run(Job.java:171)
	at org.apache.zeppelin.scheduler.RemoteScheduler$JobRunner.run(RemoteScheduler.java:326)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:178)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:292)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
	at java.lang.Thread.run(Thread.java:745)

this however works:
git clone https://github.com/apache/incubator-zeppelin.git
git fetch origin pull/208/head:208
git checkout 208
mvn package install -DskipTests
bin/zeppelin.sh

@jeffsteinmetz  A couple of things are happening here --
First, I would expect it to fail if you run it from zeppelin-distribution because the pom's aren't setup to include it in the distribution binary.  That is on the list of "to-do's," and is planned for after the initial merge.
Second, every time I've seen that error, it was because Zeppelin was configured to try to launch spark from Zeppelin home rather than from a SPARK_HOME.  It looks to me that that's what was happening here when you try to run it from the distribution directory.  (Is SPARK_HOME set?)  But the R interpreter should be detecting whether there's a misconfiguration and, if so, failing gracefully.  The reason it isn't may be because of the pom/distribution issue.  Can you email me the spark interpretergroup log of that run?
Before anyone asks -- requiring SPARK_HOME is a feature, not a bug.

ah, I see.   Well, if the pom's aren't setup to include it in the distribution binary that certainly the issue.
I didn't set SPARK_HOME intentionally (did not set up a zeppelin-env.sh).
Mainly looking to see if it worked in the distribution with the version of spark embedded in zeppelin.
Is there a sample pom file that I can test that would prepare the zeppelin-distribution binaries as expected?
from the interpreter log:
ERROR [2016-02-26 05:03:05,925] ({pool-1-thread-3} TThreadPoolServer.java[run]:296) - Error occurred during processing of message.
org.apache.zeppelin.interpreter.InterpreterException: java.lang.RuntimeException: Could not find rzeppelin - it must be in either R/lib or ../R/lib

Jeff - well that log explains it, the R package isn't being included in the distribution binary.
I'm not sure I understand either the goal or the "ask" at this point; fixing the pom's to handle building the distribution binary is planned as a post-merge to-do.  Are you saying you need this to happen pre-merge for some reason?

Nothing I need pre-merge.
I was mainly curious how close this was to working end to end.
I wasn't familiar with the pre-merge / post-merge expectations and assumed that a PR was meant to be a complete working merge.
I now see @bzz recommended keeping the distribution updates as a separate PR.
I did notice that the maven build creates a new R directory in the root of the source tree.  Is that as expected?
Curious how SPARK_HOME is used - is this ENV path used to find this R directory?  From what I understand, SPARK_HOME is meant to point to the full binary release of a standalone or cluster installation of spark (outside of the zeppelin installation location).

@elbamos please ping me if you need any help adding a profile for R in order to avoid failing default Zeppelin build for users who do not need R.

I think it is "complete" and "working" -- the convenience binary is not part of the release, and a new one isn't going to be made until there is a new release anyway.  I think the preference is to simply get the working code into HEAD and then if there are remaining issues fix them on the path to the 0.6 release.  But I certainly wouldn't object either way.
Regarding the R directory - yes.  There's a light R package created here to handle the R side of scala-R communication.  I felt that it was better to put that in a local R/ directory than to try to install it in the user's or system's R library.  That's the design choice made in Spark itself, so I tried to match that.
Regarding SPARK_HOME, yes and yes.  To talk to spark, R has to have a version of the SparkR R package that matches the running version of Spark.  That lives under the R/ subdirectory of the spark binary home, in the spark distribution.  So at startup, we need to know where the current spark binary is, so we can load the correct SparkR package.  (We could try to run without confirming the version, but in practice this generated support issues without a corresponding benefit.)
Why always SPARK_HOME and not to try to look for an R under a spark subdirectory of Zeppelin?  Because it turned out that doing that generated a torrent of support issues that arose when the user had an inconsistent Spark configuration or was running with a version of Spark other than the one they compiled against.  Part of the reason for this is that which spark installation gets used, actually is not completely defined by Zeppelin, and its changed more times than I think folks realize, including unintentionally.  (For example, compare the way the pyspark interpreter and SparkInterpreter try to locate Spark.)  Requiring that SPARK_HOME be set produces the only un-ambiguous configuration.  My understanding, from correspondence with Moon in August/September, is that spark-under-zeppelin is effectively deprecated, partly for this reason, and will soon be removed, so that Zeppelin always and only launches spark from SPARK_HOME.  So to simplify admin and the user experience, the interpreter requires SPARK_HOME, checks for it, and noisily declines to connect to spark if it isn't set.
So, to sum up -- there are a couple relevant R/ directories.  One is part of the spark distribution and contains the SparkR package for connecting R to the correct version of Spark.  There is an analogous one created for the R interpreter, under ZEPPELIN_HOME, for the rzeppelin package, which handles the R side of the Zeppelin-R interface.  During startup, we force loading these packages from those directories so we load the correct versions.
@bzz Thank you for the reminder!  I'll take care of it in the next day or so.

Makes sense
SPARK_HOME for SparkR
ZEPPELIN_HOME for the relevant R/ directories related to the R zeppelin interpreter.

I do like this approach too.
Regards,
Sourav
On Fri, Feb 26, 2016 at 7:09 AM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:

Makes sense
SPARK_HOME for SparkR
ZEPPELIN_HOME for the relevant R/ directories related to the R zeppelin
interpreter.
—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-189316874
.

@bzz Are there any tasks outstanding that you're waiting on me for?  What's left to be done for merge?
The only things I remember are outstanding, is that you were looking into both CI, and where we should put the license for files forked from other sources.

@elbamos sorry for slow re, the only things that are left:

[ ] master and this branch have diverged: we need to either merge master in here, or rebase it on top of the master. Can you look into that please?
[ ] CI failure. I volunteered to help and working on it elbamos/Zeppelin-With-R#12 Help is welcome, I hope to have more spare cycles this week to look into this one.
[ ] putting the license for files forked from other open-sourced projects in the right place (should be trivial, so can do as a final step)

@bzz - The reason a rebase is required again is that so much time has passed.  Moon committed to work on the CI issues in December.  You committed to do it a month ago.
It appears to me that essentially nothing has been done in that regard other than to add a few lines to the travis.yaml to include R.
I will rebase again tomorrow.  I propose that 208 then be merged as-is.  Whatever CI issue can be fixed later, since its not a bug in this PR anyway.

@elbamos it's totally up to you, as an author weather to do it or not. As a PPMC member I have just listed for you the requirements that project has for any contribution in order to get accepted.
If you find that my help not required here any more - please let me know.

I don't think this should output the result as a zeppelin table:
%r

mtcars

Since it's not in line with how R or R Studio responds.
It also drops the row ids, so its missing an important part of the data frame.

It can be changed.  I like verbose logging to help diagnose user issues. Logging especially the spark contection is very beneficial. But the level of logging can absolutely be reduced, or made a configuration option, whatever is the community preference.

On Mar 10, 2016, at 3:46 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
I noticed some initial verbose logging -
Package Startup Message: <packageStartupMessage in packageStartupMessage(gettextf("Loading
required package: %s",     package), domain = NA): Loading required package: knitr
Does this need to be logged to the notebook output?
—
Reply to this email directly or view it on GitHub.

This doesn't work as I would expect:
%r
print("%html <h1>HTML Works</h1>")

and %table, %md, etc
Shouldn't this work similar to other interpreters, and support the zeppelin display system?
this returns rendered html in pyspark
%pyspark

print("%html <h1>HTML Works</h1>")

as does
%spark

print("%html <h1>HTML Works</h1>")

curious why you chose to start the .z.show method calls with a period?
why not
z.show

The standard in R is that the return value of an expression is determined by the interpreter. When a data.frame is returned, the interpreter in the R distro prints a few lines; knitr formats the display differently; and Rstudio acts as a passthrough to the interpreter.
So while I'm open to doing it differently, I'm pretty sure the way it works now is actually both R-correct and Zeppelin-correct.

On Mar 10, 2016, at 3:50 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
I don't think this should output the result as a zeppelin table:
%r
mtcars
Since it's not in line with how R or R Studio responds.
It also drops the row ids, so its missing an important part of the data frame.
—
Reply to this email directly or view it on GitHub.

Does anybody have feedback regarding the use of %knitr for plotting?
Seems having to use the %knitr interpreter for certain workflows is unnecessary.
any others have feedback as to we we would want to add all of this to get graphing to work: %knitr echo=F,results='asis',message=F,warning=F
instead of just using %r
%r

library(googleVis)
Motion=gvisMotionChart(Fruits, idvar="Fruit", timevar = "Year")
print(Motion, tag='chart')

or if a generic show method was created as a helper (i realize you have .z.show.googleVis(Motion), but it feels like this could be generalized for users and we only need to deal with the %r interpreter.
%r

library(googleVis)
Motion=gvisMotionChart(Fruits, idvar="Fruit", timevar = "Year")
z.show(Motion, tag='chart')

RE: the mtcars example above, R and rstudio returns the dataframe as simple text, not a formatted table and with all of the correct row ids.
> mtcars
                     mpg cyl  disp  hp drat    wt  qsec vs am gear carb
Mazda RX4           21.0   6 160.0 110 3.90 2.620 16.46  0  1    4    4
Mazda RX4 Wag       21.0   6 160.0 110 3.90 2.875 17.02  0  1    4    4
Datsun 710          22.8   4 108.0  93 3.85 2.320 18.61  1  1    4    1
Hornet 4 Drive      21.4   6 258.0 110 3.08 3.215 19.44  1  0    3    1
Hornet Sportabout   18.7   8 360.0 175 3.15 3.440 17.02  0  0    3    2
Valiant             18.1   6 225.0 105 2.76 3.460 20.22  1  0    3    1
Duster 360          14.3   8 360.0 245 3.21 3.570 15.84  0  0    3    4
Merc 240D           24.4   4 146.7  62 3.69 3.190 20.00  1  0    4    2
Merc 230            22.8   4 140.8  95 3.92 3.150 22.90  1  0    4    2
Merc 280            19.2   6 167.6 123 3.92 3.440 18.30  1  0    4    4

In R, when a string gets returned to the repl by an expression, it gets formatted by calling print(). This is why the magic isn't working as you expect-R is adding formatting. If you write the string with cat() instead of print, it should work, and I'll fix if it doesn't.
However, in R, html is generally created using the 'html' class. If an object of class 'html' is returned by this interpreter, it is passed to Zeppelin with the %html magic for native display.
This is necessary to handle some kinds of interactive visualization. It also means the interpreter will properly display html created by R packages in the manner the user expects.
While I'm somewhat open on this issue as well, I'm pretty sure that the current design is the one that R users will find the most natural.

On Mar 10, 2016, at 3:59 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
This doesn't work as I would expect:
%r
print("%html HTML Works")
and %table, %md, etc
Shouldn't this work similar to other interpreters, and support the zeppelin display system?
this returns rendered html in pyspark
%pyspark
print("%html HTML Works")
as does
%spark
print("%html HTML Works")
—
Reply to this email directly or view it on GitHub.

The R coding style standard is that variables defined by a package are prefixed with a '.' It seemed to me that "z" stands a good chance of being overwritten as soon as, for example, the user deals with 3D data ("z <-...").
R variable scoping is weird.

On Mar 10, 2016, at 4:01 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
curious why you chose to start the .z.show method calls with a period?
why not
z.show
—
Reply to this email directly or view it on GitHub.

Many R users prefer to use knitr over a repl. Knitr also enables a lot of things that would otherwise be very complex, like packages for producing properly formatted statistical tables. (Try the papeR package and you'll see what I mean.)
Also, many other packages have hooks for knitr. If we tried to take advantage of those hooks, and the user then used knitr for anything, the functions would overwrite each other.
Otherwise we'd have to count on all the package authors to write their own display code specific to Zeppelin.

On Mar 10, 2016, at 4:09 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
Does anybody have feedback regarding the use of %knitr for plotting?
Seems having to use the %knitr interpreter for certain workflows is unnecessary.
any others have feedback as to we we would want to add all of this to get graphing to work: %knitr echo=F,results='asis',message=F,warning=F
instead of just using %r
%r
library(googleVis)
Motion=gvisMotionChart(Fruits, idvar="Fruit", timevar = "Year")
print(Motion, tag='chart')
or if a generic show method was created as a helper (i realize you have .z.show.googleVis(Motion), but it feels like this could be generalized for users and we only need to deal with the %r interpreter.
%r
library(googleVis)
Motion=gvisMotionChart(Fruits, idvar="Fruit", timevar = "Year")
z.show(Motion, tag='chart')
—
Reply to this email directly or view it on GitHub.

The row id's aren't part of the table in R. Also, R actually is formatting the table in a specific way. Try returning a data_frame vs a data.frame in R and see what happens.
If the user wants the "raw text" version, they simply call print() on the table to turn it into a string.
This is a Zeppelin feature!

On Mar 10, 2016, at 4:13 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
RE: the mtcars example above, R and rstudio returns the dataframe as simple text, not a formatted table and with all of the correct row ids.

mtcars
mpg cyl  disp  hp drat    wt  qsec vs am gear carb
Mazda RX4           21.0   6 160.0 110 3.90 2.620 16.46  0  1    4    4
Mazda RX4 Wag       21.0   6 160.0 110 3.90 2.875 17.02  0  1    4    4
Datsun 710          22.8   4 108.0  93 3.85 2.320 18.61  1  1    4    1
Hornet 4 Drive      21.4   6 258.0 110 3.08 3.215 19.44  1  0    3    1
Hornet Sportabout   18.7   8 360.0 175 3.15 3.440 17.02  0  0    3    2
Valiant             18.1   6 225.0 105 2.76 3.460 20.22  1  0    3    1
Duster 360          14.3   8 360.0 245 3.21 3.570 15.84  0  0    3    4
Merc 240D           24.4   4 146.7  62 3.69 3.190 20.00  1  0    4    2
Merc 230            22.8   4 140.8  95 3.92 3.150 22.90  1  0    4    2
Merc 280            19.2   6 167.6 123 3.92 3.440 18.30  1  0    4    4
—
Reply to this email directly or view it on GitHub.

I just want to reiterate, I am open on these issues. But who I think we should look to hear from regarding user expectations, are people who are regular users of R.
The R ethos is to follow the intuitions of a statistician, even for choices, like indexing arrays from 1 or not having scalars, seem nonsensical to computer scientists. There are books on this subject.

On Mar 10, 2016, at 4:28 PM, elbamos<EMAIL_ADDRESS>wrote:
The row id's aren't part of the table in R. Also, R actually is formatting the table in a specific way. Try returning a data_frame vs a data.frame in R and see what happens.
If the user wants the "raw text" version, they simply call print() on the table to turn it into a string.
This is a Zeppelin feature!

On Mar 10, 2016, at 4:13 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
RE: the mtcars example above, R and rstudio returns the dataframe as simple text, not a formatted table and with all of the correct row ids.

mtcars
mpg cyl disp hp drat wt qsec vs am gear carb
Mazda RX4 21.0 6 160.0 110 3.90 2.620 16.46 0 1 4 4
Mazda RX4 Wag 21.0 6 160.0 110 3.90 2.875 17.02 0 1 4 4
Datsun 710 22.8 4 108.0 93 3.85 2.320 18.61 1 1 4 1
Hornet 4 Drive 21.4 6 258.0 110 3.08 3.215 19.44 1 0 3 1
Hornet Sportabout 18.7 8 360.0 175 3.15 3.440 17.02 0 0 3 2
Valiant 18.1 6 225.0 105 2.76 3.460 20.22 1 0 3 1
Duster 360 14.3 8 360.0 245 3.21 3.570 15.84 0 0 3 4
Merc 240D 24.4 4 146.7 62 3.69 3.190 20.00 1 0 4 2
Merc 230 22.8 4 140.8 95 3.92 3.150 22.90 1 0 4 2
Merc 280 19.2 6 167.6 123 3.92 3.440 18.30 1 0 4 4
—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub.

print(mtcars)
returns text in a similar way as R as you suggested.
Perhaps some hints in the interpreter docs.
Not seeing the row names as the default zeppelin output was perhaps unexpected behavior for me – I would say I am a moderately frequent user of R.
I don’t live in it every day though.
It does make sense that row names are really not part of the data frame columns.
If somebody wants the row names as a column, they can
myDF <- cbind(Row.Names = rownames(myDF), myDF)
From:  elbamos
Reply-To:  apache/incubator-zeppelin
Date:  Thursday, March 10, 2016 at 1:28 PM
To:  apache/incubator-zeppelin
Cc:  Jeff Steinmetz
Subject:  Re: [incubator-zeppelin] R Interpreter for Zeppelin (#208)
The row id's aren't part of the table in R. Also, R actually is formatting the table in a specific way. Try returning a data_frame vs a data.frame in R and see what happens.
If the user wants the "raw text" version, they simply call print() on the table to turn it into a string.
This is a Zeppelin feature!

On Mar 10, 2016, at 4:13 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
RE: the mtcars example above, R and rstudio returns the dataframe as simple text, not a formatted table and with all of the correct row ids.

mtcars
mpg cyl  disp  hp drat    wt  qsec vs am gear carb
Mazda RX4           21.0   6 160.0 110 3.90 2.620 16.46  0  1    4    4
Mazda RX4 Wag       21.0   6 160.0 110 3.90 2.875 17.02  0  1    4    4
Datsun 710          22.8   4 108.0  93 3.85 2.320 18.61  1  1    4    1
Hornet 4 Drive      21.4   6 258.0 110 3.08 3.215 19.44  1  0    3    1
Hornet Sportabout   18.7   8 360.0 175 3.15 3.440 17.02  0  0    3    2
Valiant             18.1   6 225.0 105 2.76 3.460 20.22  1  0    3    1
Duster 360          14.3   8 360.0 245 3.21 3.570 15.84  0  0    3    4
Merc 240D           24.4   4 146.7  62 3.69 3.190 20.00  1  0    4    2
Merc 230            22.8   4 140.8  95 3.92 3.150 22.90  1  0    4    2
Merc 280            19.2   6 167.6 123 3.92 3.440 18.30  1  0    4    4
—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub.

To confirm, are you saying this should return Zeppelin displayed html? (I may have misunderstood)
%r

cat("%html <h1>hello!</h1>”)

If so, it outputs text as is (which still makes sense based on your explanation, since its not an HTML class?)
%html hello!
Do you have an example of how to create an HTML class in the interpreter?
As an aside, while testing this I ran across a locked environment error
setClass("Person", representation(name = "character", age = "numeric"),
  prototype(name = NA_character_, age = NA_real_))

Returns an error:
<simpleError in assign(mname, def, where): cannot add bindings to a locked environment>
From:  elbamos
Reply-To:  apache/incubator-zeppelin
Date:  Thursday, March 10, 2016 at 1:18 PM
To:  apache/incubator-zeppelin
Cc:  Jeff Steinmetz
Subject:  Re: [incubator-zeppelin] R Interpreter for Zeppelin (#208)
In R, when a string gets returned to the repl by an expression, it gets formatted by calling print(). This is why the magic isn't working as you expect-R is adding formatting. If you write the string with cat() instead of print, it should work, and I'll fix if it doesn't.
However, in R, html is generally created using the 'html' class. If an object of class 'html' is returned by this interpreter, it is passed to Zeppelin with the %html magic for native display.
This is necessary to handle some kinds of interactive visualization. It also means the interpreter will properly display html created by R packages in the manner the user expects.
While I'm somewhat open on this issue as well, I'm pretty sure that the current design is the one that R users will find the most natural.

On Mar 10, 2016, at 3:59 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
This doesn't work as I would expect:
%r
print("%html HTML Works")
and %table, %md, etc
Shouldn't this work similar to other interpreters, and support the zeppelin display system?
this returns rendered html in pyspark
%pyspark
print("%html HTML Works")
as does
%spark
print("%html HTML Works")
—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub.

Yes you did understand - I just checked and I know why it didn't work for you, I'll fix it next time I'm in the code.
The xtable package generates html, as a simple example.  I will confess, I haven't tested the html functionality in quite a while-the reason is, anyone who wants html displayed, i can't think of a reason they wouldn't use the knitr interpreter. But it should work.
Regarding the locked environment error, it seems like you've loaded a package that has a dependency on a package that defines a "Person" object.  What happens when search the namespaces for Person?  But honestly I've never seen that error before.

On Mar 10, 2016, at 5:46 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
To confirm, are you saying this should return Zeppelin displayed html? (I may have misunderstood)
%r

cat("%html <h1>hello!</h1>”)

If so, it outputs text as is (which still makes sense based on your explanation, since its not an HTML class?)
%html hello!
Do you have an example of how to create an HTML class in the interpreter?
As an aside, while testing this I ran across a locked environment error
setClass("Person", representation(name = "character", age = "numeric"),
prototype(name = NA_character_, age = NA_real_))

Returns an error:
<simpleError in assign(mname, def, where): cannot add bindings to a locked environment>
From: elbamos
Reply-To: apache/incubator-zeppelin
Date: Thursday, March 10, 2016 at 1:18 PM
To: apache/incubator-zeppelin
Cc: Jeff Steinmetz
Subject: Re: [incubator-zeppelin] R Interpreter for Zeppelin (#208)
In R, when a string gets returned to the repl by an expression, it gets formatted by calling print(). This is why the magic isn't working as you expect-R is adding formatting. If you write the string with cat() instead of print, it should work, and I'll fix if it doesn't.
However, in R, html is generally created using the 'html' class. If an object of class 'html' is returned by this interpreter, it is passed to Zeppelin with the %html magic for native display.
This is necessary to handle some kinds of interactive visualization. It also means the interpreter will properly display html created by R packages in the manner the user expects.
While I'm somewhat open on this issue as well, I'm pretty sure that the current design is the one that R users will find the most natural.

On Mar 10, 2016, at 3:59 PM, Jeff Steinmetz<EMAIL_ADDRESS>wrote:
This doesn't work as I would expect:
%r
print("%html HTML Works")
and %table, %md, etc
Shouldn't this work similar to other interpreters, and support the zeppelin display system?
this returns rendered html in pyspark
%pyspark
print("%html HTML Works")
as does
%spark
print("%html HTML Works")
—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub.
—
Reply to this email directly or view it on GitHub.

@bzz   Can you update us on the status of this?  What is the ETA on making progress with CI?

From what I can see - same three issue I have listed above still hold for this PR from being merged.
Updates on my help with CI are tracked under elbamos/Zeppelin-With-R#12
@elbamos your comments there implied that you are not familiar with project's build\QA process. It's not very documented I admit, but is pretty standard for java project under asf, so I would advise you to either familiarize yourself with it, or wait until somebody volunteers to help you with this one, on their own spare time.
Both CI issues from elbamos/Zeppelin-With-R#12 do not exist on master, and both are reproducible from this branch on my local env.  That clearly implied not a project-wise problem, but something related only to this branch.
I will be looking further on how to help you here, and assume that it is something appreciated rather taken as "granted". Remember, we all have daytime jobs, families, etc.
But please feel free to politely stop me, as soon as you feel that my help here is no longer required.

@bzz Alex - The two "issues" that you identified on my repo are as follows:

Two files are flagged as requiring an Apache license header --- these are files where you had undertaken to determine where we should put the license, because we agreed that a header is not appropriate.   That is your action item, and its been outstanding for six weeks.

PySpark fails its integration tests with Spark.  The version of this PR on my repo contains a very small change to PySpark, to get the spark home correctly.  The version here never touches PySpark.

I have not been able to replicate the problem you cut-and-pasted, in any environment, from either PR version.
If you click the Travis link just above, you will see that its green except for 1.6 and 1.2.2.  It does not appear to produce the error you cut and pasted.

I am certainly sympathetic to the fact that we all have day jobs and families.
In this case, working on Zeppelin is your day job -- you're an NFlabs employee.  Since you were an early author, one would presume that you also have a personal financial interest in it.  NFLabs submitted this project to Apache for commercial reasons.
This has been outstanding since August.  When you undertook to work on this, we got off to quite a good start, and I was very optimistic.
At this point, so far, it appears that what you have done is ask me four times to repeatedly list the PR's R package dependencies -- which you never even used.
I'm sure you could see why, at this point, I would have serious doubts about whether your engagement has been bona fide at all.
P.S.:  If you think I don't understand java build, I'm sure you will be happy to identify each and every error I've made.

@bzz Let me just add, to avoid any doubt --- I hope I'm wrong.  If I am, then I'll be most appreciate, and most apologetic for misjudging you.
In that case, I'm sure you'll be able to understand why, at this point, from my perspective, my trust level is pretty low.

The push earlier tonight should (if it doesn't, let me know) resolve all comments I've received about this PR.
It currently passes CI for Spark 1.6.  The other Spark variants fail on the PySpark tests identified by Alex.  Given that Zeppelin's PySpark integration changed in 1.6, and that that change has been a source of several hot-fixes already, the PySpark changes would seem to be the culprit.

Things that left to be done in order to merge this PR:

[x] master and this branch have diverged: we need to either merge master in here, or rebase it on top of the master
[ ] CI failure. I volunteered to help and working on it elbamos/Zeppelin-With-R#12 Help is welcome, I hope to have more spare cycles this week to look into this one.
[ ] putting the license for files, forked from other open-sourced projects in the right place. Having a list those files would help, see this @jeffsteinmetz comment.

Should the interpreter list %r in the header?
Right now it shows %knitr, but not %r
spark %spark (default) , %sql , %knitr , %pyspark , %spark , %dep

As a reminder, building the distribution with -Pbuild-distr doesn't package the interpreter as expected.

@jeffsteinmetz I had a typo in the default template, it should be fixed now and %r show up.  Thanks for catching!
You are correct that build-distr won't include this --- when we made the work plan for getting the PR merged, we had a consensus to take that item and deal with it separately, so we could get the PR merged and move on as soon as possible.

I just spent 3 hours trying to replicate the error.   I cannot replicate it on Ubuntu or OSX with Spark 1.5.2 or Spark 1.6.
Looking at the error closely, CI is not making Spark available to the testing suite, which is essentially the issue it's been since September.

@elbamos :  One thought:  The -Ppyspark build profile will download the spark distribution in order to pass tests and include pyspark in build the distribution with all required spark dependencies.
Is there an equivalent for this PR to work with CI?
It might also lead to some benefits for the future -Pbuild-distr work.

@jeffsteinmetz I'm not sure I understand what you're proposing?
At this moment this PR isn't failing CI.  It builds and tests fine.
When CI later tries to use a recently-added test class that has nothing to do with this PR, the class tries to talk to Spark, but Spark isn't running.

@elbamos : It may be unrelated.  I didn't take a close look at the specific CI failure - if its unrelated, please disregard.

@jeffsteinmetz  They're all unrelated.  That's kind of the point.
A few minutes ago it was passing Spark 1.6 and selenium, but failing everything else.  Now, it passes everything else, but fails Spark 1.6 and selenium.  The only difference is that I removed one of the REST testing classes.

@elbamos removing test that do not pass under b848d98 is not an acceptable way to make CI green.
It is not going to be possible to merge it without ZeppelinSparkClusterTest.java I'm affraid.
Please put it back and let's fix the reason it is failing on this branch.

@bzz Do you intend to take any constructive action to move this forward?  This PR has been waiting for your action items for three weeks, where you have done nothing but make derogatory comments toward me.
I did not intend to take the class out permanently. I took it out to see what would happen.    Look at what did happen:  The tests that fail with ZeppelinSparkClusterTest there, pass.  But the tests that passed with ZeppelinSparkClusterTest, fail after its removed.
If the test is generating spurious failures, then the test needs to come out of Zeppelin regardless of this PR.  This part of the testing infrastructure is your code, right?
Either way, I'm sure you can be helpful if you choose to be.

@elbamos I see. The problem is that such intentions (removing not permanently) are not obvious for a reviewers, as they just do not know and assume that current state is the one you want to get merged.
Experiments are very good, but if you conduct them in a separate from this PR branch (but on top this work, same way as  elbamos/Zeppelin-With-R#12) - it would simplify the final review of this PR a lot, making things faster.
I.e tests generating spurious failures must be proven to do so on other PRs\branches and indeed must be improved or taken out, but all this should happen outside of the scope of this particular PR, keeping things small and simple. (I.e if one test is found to be guilty, the PR fixing\removing it must be merged first and then this one rebased on top)
That is what I meant. Does it make sense?

@bzz It does make sense. Tom's proposal, which everyone liked, was to merge this into a feature version branch, and we can all use that to track down the CI issue.
One of you needs to that -- I can't.
Also -- will you be looking at the causes of these failures?  It appears to me that this is exactly the same issue as its been all along, which is that Spark does not reliably launch in CI.

@elbamos It is possible to do exactly so, but then we will need to create a new PR from that feature branch to master, in order to get it merged, right?
And this way, that PR will not be under your name (so tooling probably will not pick it up on final merge), also ASF<->GH integration does not grant full rights to committers either, i.e not sure that we will be able to create such PR ->master, as we never tried. What I'm saying is - it might take some time to figure that out, as this is a bit different from the regular process the project followed before.
What do you think would be better?

treat this branch as "Feature branch" and freeze it, making all CI work on top of it (as PRs to your fork)
or create a "feature branch" under apache repo, new PR -> master, and PRs with CI work on top of that one
or create a new "feature branch" under your repo and PRs for CI from there

Let me know what you think!
I'm willing to help nailing down the pyspark tests failures - spark been not reliably launched on CI can be the case indeed, but it is way too "reproducible" - somehow if fails every single time, so it may be just some very simple but un-obvious bug somewhere.

@bzz I don't know what the implications are of those options.
Honestly, I feel like if we spend time on that, its going to be a distraction, because I have a suspicion that if there is a non-obvious bug here, and you wrote the testing infrastructure, that the bug is going to be found 15 minutes after someone starts looking.
Can you take a look at what's going on in the push when I removed the test class?  What concerns me here is not that the tests that had failed, started to pass --- its that the tests that had passed before, started to fail after the test was deleted.

So you suggest to keep CI work in this branch is try to fix current issues first and then get back to ZeppelinSparkClusterTest.java - it's fine with me.
I would suggest we divide CI work "by profile", and deal with it one by one.

@bzz The right place for the discussion of whether to work in this PR or a branch or whatever, if Tom's proposal is off the table, is the developers' mailing list.
Regarding how to divvy up the work -- it seems like there's "1.6" and "everything else."  Which one do you want to pick, and when?

I'm not sure I understand you, but please feel free to use whatever place you consider right for the any discussion you feel necessary (I would suggest starting a new thread with the descriptive subject though).
Here is the list of things that need to be done in order to merge this PR:

[x] master and this branch have diverged: we need to either merge master in here, or rebase it on top of the master
[ ] revert deletion of ZeppelinSparkClusterTest.java
[ ] make CI green. I suggest attack each profile "one by one", creating a new JIRA issue for each and every new failure, see ZEPPELIN-774 as an example.
[ ] putting the license for files, forked from other open-sourced projects in the right place. Having a list those files would help, see this @jeffsteinmetz comment.

Open Jira issues for each failed test?  It looks that you are just wasting time.  These are all issues that you committed, to me and to the community, that you would do work on, months ago.
So far, what you've done is to, again and again, make lists of the things that you haven't done, while lobbing insults at me.
Licenses:  We have waited months for you to simply decide a rule for where the licenses will go for forked files.  Only a PMC member can make that decision.  Once it is made, I will put the licenses in the right place.  If you want a list, you can use the list that we made previously, which is on this very page.
CI:  The question is what is the quickest path for you to actually examine whether these errors are anything other than bugs in the testing infrastructure.  If you want to divvy things up, i don't care.  What I do care about, is that you actually begin to do the things that you committed to do.
Alternatively, we can go back to the consensus of the community, which is simply to merge it.
I'm confident that any of these other issues will be resolved within minutes of accepting that.

I do not see how this words help. Please ping me when any of the issues above are solved and you want me to spend time reviewing this PR again.

Its green now, Alex.
ZeppelinSparkClusterTest.java is broken.  I will try to fix it, but it
isn't required that I debug broken tests of completely different parts of
Zeppelin in order to merge this PR.
On Wed, Mar 30, 2016 at 1:12 AM, Alexander<EMAIL_ADDRESS>wrote:

I do not see how this words help. Please ping me when any of the issues
above are solved and you want me to spend time reviewing this PR again.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-203251582

Great!
Here is the list of things that need to be done in order to merge this PR:

[x] master and this branch have diverged: we need to either merge master in here, or rebase it on top of the master

[ ] revert deletion of ZeppelinSparkClusterTest.java

[ ] make CI green
I will be looking into ZEPPELIN-776 today, will post here. If I could not identify the reason today - please feel free to user JUnit Ignore to take it out of the test suite in this PR, instead of deleting other people's contributions

[ ] put the license for files, forked from other open-sourced projects in the right place

@bzz I will look into junit ignore.  Regarding the licenses, that issue is where it has been:  For files that are forked from other things, but also contain changes, I need to know where you would like the licenses to go.
What is necessary is simply a decision.    We can't just put the apache header in those files, because some of the content is covered by another license.  Currently those files are identified in the "other licenses" file -- but, the portion of the files that has changed, is covered by the Apache license.
So, again, what's necessary here is simply the decision, and because it is a licensing issue, it cannot be made by anyone but a member of the PPMC.

@elbamos right, I'm looking into licenses and will post back soon. Meanwhile can you please help providing the list of such files to save the time identifying them? I have updated the comment above to my knowledge, but that may be not everything.

@bzz - that's basically the list for scala files, except that I'm pretty sure RException was mine.  The other thing is the R source.  The rzeppelin.R is mine.  Other than that, the R files are either directly from rscala, or substantially not changed.

@bzz - The test is back in with @ignore, and CI is green.  Do you have a decision on the license files?

On RException - then please feel free to remove //Originally by David Dahl and released under the BSD license if that is your contribution.

Meanwhile can you please help providing the list of such files to save the time identifying them?

As there is no list yet, it will take some time to me to make it for you - I will post back as soon as it's ready so you can review and let me know if it's correct.
After that, it will take some time to review the whole PR again, as there were many commits recently.
Then we will go through the list of files one by one, to identify whether a substantial portions of the file was changed or not

@elbamos I have create file list and updated comment above to track our progress https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-203673589
Is that the full list? Could you please help answering the questions there, so we could clearly identify licence for each of them.

@bzz Thank you for catching RException - I have taken out the reference to DDahl.
I don't think this is that complicated:

RClient.scala --- Modified from the original, this is unhandled
Console.scala --- Originally from scala distribution, unhandled
r/R/rzeppelin/R/*.R (except for rZeppelin.R) ---  These are essentially unchanged from rscala.

That's it.

@elbamos sure, now I have updated list in https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-203673589 could you please clarify few question left there in the comments linked from the list?

@bzz I just looked at the list, and its all the same thing.  I took out of rscala the code for making an R->scala connection.  There isn't anything new added, or anything reorganized, etc., so there's nothing to assert a new copyright interest in.  It's all BSD -- its just that not everything that was in the original file is included.

re Docs: I noticed this docs/interpreter/screenshots/knitmotion.png and a few other screen shots no longer reflect the current implementation.

@jeffsteinmetz Can you be more specific?  Do you mean because they refer to %spark.r and %spark.knitr?

Ah - you mean because I took out the show() functions yesterday in respect of your comment about that.

The .z.show.googleVis etc.

@elbamos got it, but could you please post your answers in the comments that I have made, and linked from the list, so we have full conversation history online?
Please keep in mind that I'm asking not because I want so, but because when somebody later on will be curious about the decision we made here, he could just follow the links and read your answers, without the need to follow all hundreds of comments in this thread.
So it will just save the time for future reviewers, that's it. Hope this make sense.

@bzz I'm not sure I understand what you're requesting -- but in any event, I think the issue for right now is resolved, correct?

@jeffsteinmetz I just pushed the fixed screenshots.  Thanks, by the way, for asking that the show functions be taken out --- I can see that the googleVis package has been updated so that they're no longer necessary; when I wrote this, it was actually somewhat tricky to get the visualizations to show up inline, but that's no longer the case.

@elbamos :+1: for keeping the docs update! Yes, I believe licenses for each contributed file are clear now, but what I was asking you for is just to follow each please clarify link from the https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-203673589 where I have posted a question for you about modification\licences and post your ansewers there.
It may look like a formality but from my experience such things will allow us to keep history of this important conversation and it will simplify the review for those, who are interested to follow it. Would that be something you are willing to do? Thank you in advance.
Also please mind, as this is a first changeset that includes so many files from different places contributed to project codebase, I have also asked IPMC to verify the correctness of our interpretation of the legal aspect, so please be patient, we are almost there, I would expect it will take no more than a day or two to advise on this matter.

@bzz I'm not questioning the wisdom of why you're asking me what to do what you're asking -- I just literally do not understand what you're asking. When you've made a "please clarify" request, I've either been responding to the e-mail, or posting it as a line note.  What is it, exactly, that you're asking?
I will write you privately regarding the rest of your response.

@elbamos I'm sorry for a confusion! I was basically asking you to post same replies that you did before, but in the places where I have posted the questions - comments to particular files, so they can be found later. They are all linked from https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-203673589 and marked "please clarify", you can just follow each link and respond by commenting there. It's basically comments on the files, starting from r/R/rzeppelin/R/common.R and a bit down.
I also would very much prefer keep the project communication on the list (this thread is mirrored on the list) - so please post here or on the dev@ mailing list directly, if my feedback is required.

@bzz I think I've just done what you requested.

@elbamos thank you, it looks great now.
Could you please take care of the last step - updating LICENSE files, while I'll focus on ZEPPELIN-776 ?
Here is updated list, just a few things left

[x] master and this branch have diverged: we need to either merge master in here, or rebase it on top of the master

[x] revert deletion of ZeppelinSparkClusterTest.java

[ ] make CI green
It's almost done, except for ZEPPELIN-776. I will be looking into today\tomorrow and will post here. If we could not identify the reason today - please feel free to user JUnit Ignore to take it out of the test suite in this PR

[x] determine the license for files, forked from other open-sourced projects

[x] r/src/main/scala/scala/Console.scala - scala BSD
[x] r/src/main/scala/org/apache/zeppelin/rinterpreter/rscala/Package.scala rscala BSD
[x] r/src/main/scala/org/apache/zeppelin/rinterpreter/rscala/RClient.scala rscala BSD
[x] r/R/rzeppelin/R/common.R rscala BSD
[x] r/R/rzeppelin/R/globals.R rscala BSD
[x] r/R/rzeppelin/R/protocol.R rscala BSD
[x] r/R/rzeppelin/R/rServer.R rscala BSD
[x] r/R/rzeppelin/R/scalaInterpreter.R rscala BSD
[x] r/R/rzeppelin/R/zzz.R rscala BSD

[ ] update LICENSE files

[ ]  add 2 new files under licenses/LICENSE-[project]-[version].txt with full text of the above licenses (for rscala - bolerplate text from https://cran.r-project.org/web/licenses/BSD_3_clause with 3 lines replaced from https://cran.r-project.org/web/packages/rscala/LICENSE, http://www.scala-lang.org/download/#License for Scala)
[ ] list each license + list of file paths in ./LICENSE

========================================================================
BSD-style licenses
The following components are provided under the BSD-style License. See project link for details.
The text of each license is also included at licenses/LICENSE-[project]-[version].txt.
(BSD 3 Clause) next portions of rscala 1.0.6 (https://dahl.byu.edu/software/rscala/) - https://cran.r-project.org/web/packages/rscala/index.html
r/R/rzeppelin/R/{common.R, globals.R  }
r/src/main/scala/org/apache/zeppelin/rinterpreter/rscala/{Package.scala, RClient.scala}
(BSD 3 Clause) next portions of Scala http://www.scala-lang.org/download/#License ...
r/src/main/scala/scala/Console.scala
```
- [ ] for now, remove rscala from ./zeppelin-distribution/src/bin_license/LICENSE

Is it good to merge?

I need to take care of Alex' license requests, which I will later this morning.

On Apr 1, 2016, at 2:10 AM, Ankur Jain<EMAIL_ADDRESS>wrote:
Is it good to merge?
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@ankurmitujjain please refer https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-203834473 for things that need to be taken care of before merging

@bzz Here's where we are:

I believe all the licensing, rat, and rebasing to-do's are resolved.
On CI, trying to follow your proposal, if we turn-off sudo in CI, then the pyspark tests pass.

BUT To test the R interpreter, we need to be able to install the R dependencies.
The base installation directory for R libraries is owned by root and not writable, so we need sudo to change permissions.
The method used in 702 is to use a user-specific R preference file to store R packages in a non-standard location.  We can't use that here, because the R interpreter ignores user-specific preferences and directories --- this is an intentional design decision.

I am experimenting with a few options -- do you have suggestions?

@elbamos A minor comment:
sudo type of solution won't work on windows, of course.
From a R perspective, the alternatives I can see are:

write packages into the user package library.  This is what RStudio does with packages like rstudioapi or manipulate, installed (and updated) into the user package library in a totally transparent way by RStudio
create a "dedicated", Zeppelin own, library.  The solution should make sure that any package loaded or installed by the user is kept "separate" form this dedicated library (for consistency and error avoidance).

Yeah, I may let it load from the Zeppelin R library if we can't find another solution.
I really don't want to use a user-specific directory, because I think we're going to end up with library conflicts related to sparkR.
As for Windows --- are we supporting Windows outside of docker?

On Apr 1, 2016, at 6:06 PM, Enzo<EMAIL_ADDRESS>wrote:
@elbamos A minor comment:
sudo type of solution won't work on windows, of course.
From a R perspective, the alternatives I can see are:
write packages into the user package library. This is what RStudio does with packages like rstudioapi or manipulate, installed (and updated) into the user package library in a totally transparent way by RStudio
create a "dedicated", Zeppelin own, library. The solution should make sure that any package loaded or installed by the user is kept "separate" form this dedicated library (for consistency and error avoidance).
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

We may be good now - the only failures on the last push were of the CI-random type.

@bzz are we good?

@elbamos I have switched to the containerized sudo-less method of R installation + dependencies in elbamos/Zeppelin-With-R#12 it works, let me rebase and we should be able to merge it in.
The only problem is now un-relaible devtools::install_github('IRkernel/repr')...

I believe that rebase to latest master must eliminate most of CI issues.
Did one more round of reviews and updated https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-203834473 - a minor cleanup of licenses is needed.

@bzz We are green!  Good to go?

Looks awesome! I think there are just 2 minor licence issue that are left in the ..
Copied them here for convenience

@bzz The push I just made should reflect all of the latest requests. Thank you for pointing out the effect of a case-insensitive FS.  There are no substantive changes in this push, so if its going red, its just the random-CI stuff.

@bzz Are we good?

@elbamos awesome, thank you! Please, let me do a final pass and I'll post back ASAP.
Just FYI - I'm still working on fixing random CI failures under ZEPPELIN-783 (not related\blocking this work)

@bzz thanks!
Yeah, I was going to suggest, that getting CI to work reliably is probably something important to the project beyond the scope of this PR.
One thing that might offer a quick improvement, is @jeffsteinmetz's suggestion that we stop supporting older versions of Spark (e.g., 1.1, 1.2).  That should reduce the random test failures by at least 20%.

@elbamos everything looks good, except one last thing - BSD license text need to be removed from r/src/main/scala/scala/Console.scala header as well, as it was not there originally and we have it in a separate file, clearly pointed from root LICENSE.
Sorry for not pointing it out earlier, but reviewing whole 4000+LOC diff everytime at once is a bit uneasy.
I think we should be good with merging it right after that. And yes, improving project CI is a priority and will be continued.

@bzz So we should be good to merge now right?

Looks good to me. thank you for the efforts.
I think it is ready to be merged, and if there is no more discussion - I will merge it tomorrow.

@bzz the push I just made fixed an issue that if -Pr wasn't specified, rat would complain about the licenses of files under the r/ subdirectory.  There are no other changes.

+1