I have also experienced CI failing in the pass for some PRs that do not impact the code (Cassandra documentation). I guess that during peak hours, the CI servers may be too busy or enter a dead lock so the build fails.
At least that's my guess. What do you think ? On Tue, Dec 8, 2015 at 9:42 AM, moon soo Lee <[email protected]> wrote: > Let me run some CI test with your branch and share result here. > Hope i can narrow down the cause and that helps involvement of more people. > > Thanks, > moon > > On Tue, Dec 8, 2015 at 3:08 PM Amos B. Elberg <[email protected]> > wrote: > > > Is there anyone who can work with me on the CI issues? It looks like > > there are a number of PRs experiencing similar things. > > > > I think we should make getting CI stable to be a priority. Because it > > will save everyone a lot of frustration and aggravation if CI works > > reliably. > > > > Is there anyone other than Jongyoul and Moon who knows the CI/Build > > process well? > > > > (Moon - Thank you for taking another look at the licensing issue. Per > the > > e-mail I wrote about this a few days ago, I don’t feel I have more to > > contribute to the licensing discussion, so I’m going to try not to > comment > > further about it.) > > > > From: moon soo Lee <[email protected]> <[email protected]> > > Reply: [email protected] > > <[email protected]> <[email protected]> > > Date: December 7, 2015 at 5:00:08 PM > > To: [email protected] <[email protected] > > > > <[email protected]> > > Subject: Re: License of KnitRInterpreter Was: Re: contributions impasse. > > Was: [GitHub] incubator-zeppelin pull request: R Interpreter for Zeppelin > > > > Thanks Amos, Roman, Cos for clarifying license issue. > > > > I'm convinced that this license issue will not be a blocker. > > > > In my understanding, these are good sign, > > > > 1. any gpl licensed source codes are not included in the source package > > 2. any gpl licensed libraries are not included in the binary package > > > > However, i can not still 100% sure about > > > > 3. any gpl licensed libraries are not linked on runtime > > > > Even after Amos's explanation. I still think using 'knitr' is one of the > > clear case that 'knir' is linked to 'R' according to > > http://www.gnu.org/licenses/gpl-faq.en.html#IfInterpreterIsGPL. > > > > Giving input and getting output from GPL licensed interpreter (includes > R) > > from Apache licensed software is not a problem. That's not the point. > > Let me say in this way. There's an java code, > > > > import com.mysql.jdbc.Driver > > Driver driver = new Driver() > > > > Say without this java code, one of important feature of Zeppelin does not > > work. And Zeppelin does not includes GPL licensed file in the source > > package, GPL licensed library in the binary package, but it requires GPL > > licensed library on the runtime. > > In this case, will this java code be a license problem or not? > > > > In other words, my question is > > > > a) Is runtime GPL library dependency allowed in ASF release? > > b) is 'knitr' considered as runtime dependency? > > > > If someone can clarify a), b), then it would be extremely helpful > > understanding this case, and possible similar cases, too. > > > > Thanks, > > moon > > > > On Mon, Dec 7, 2015 at 3:20 PM Konstantin Boudnik <[email protected]> > wrote: > > > > > On Thu, Dec 03, 2015 at 11:03AM, Corneau Damien wrote: > > > > Thanks Cos for those answers, > > > > > > > > If I'm right you are advising to have a default build that doesn't > > > include > > > > libraries with conflicting licenses, but have an option to include > > them > > > for > > > > users who wants to build the project themselves. > > > > > > Yes, that's what I said. Besides, looks like Roman provided the second > > > pair of > > > eyes to this licensing discussion and as well didn't find any issues > > with > > > the > > > current approach. > > > > > > Cos > > > > > > > To refer to another thread about decentralizing interpreters, it > could > > > even > > > > be better in that case to have some interpreters separated from the > > tree, > > > > and easily pluggable with a release instead of forcing users to build > > the > > > > project to use them. > > > > > > > > > > > > On Thu, Dec 3, 2015 at 9:26 AM, Konstantin Boudnik <[email protected]> > > > wrote: > > > > > > > > > On Wed, Dec 02, 2015 at 04:28PM, Amos B. Elberg wrote: > > > > > > Konstantin thank you for getting into this. > > > > > > > > > > > >> The best way to go around it is by > > > > > >> providing a build-time option that will pull such binaries in. > > But > > > by > > > > > default > > > > > >> such libs shouldn't be pulled. > > > > > > > > > > > > That is basically how the PR handles this. If the GPL’d > > interpreter > > > > > scripts > > > > > > are missing, there’s no indication at all at build time. It > > doesn’t > > > try > > > > > to > > > > > > download them. > > > > > > > > > > > > At runtime, if the user tries to use functionality that would > need > > > such a > > > > > > script (i.e., if they type “knitr” to use knitr), we display an > > error > > > > > that > > > > > > says that the functionality is not there because the library is > > > missing, > > > > > and > > > > > > that the library cannot be provided because it has an > incompatible > > > > > license, > > > > > > but the user can download it themselves if they want. > > > > > > > > > > > > And, in the log, if the logging level is high, they will see a > > note > > > that > > > > > > some functionality was disabled because the libraries weren’t > > there. > > > > > > > > > > > > To be clear, though, none of these libraries are binaries. > They’re > > > all > > > > > interpreter scripts. > > > > > > > > > > If you ever in a doubt of which licenses could be used for > > dependncies > > > > > (not to > > > > > say about source code) are listed in Category A list of [1] > > > > > > > > > > A lot of quesitons discussed here are already covered in the legal > > > FAQ, so > > > > > just check against it if you have any questions. > > > > > > > > > > [1] http://www.apache.org/legal/resolved.html#category-a > > > > > > > > > > Cos > > > > > > > > > > > From: Konstantin Boudnik <[email protected]> > > > > > > Reply: [email protected] < > > > > > [email protected]>, > > [email protected] > > > < > > > > > [email protected]> > > > > > > Date: December 2, 2015 at 3:24:50 PM > > > > > > To: [email protected] < > > > [email protected] > > > > > > > > > > > > Subject: Re: License of KnitRInterpreter Was: Re: contributions > > > > > impasse. Was: [GitHub] incubator-zeppelin pull request: R > > Interpreter > > > for > > > > > Zeppelin > > > > > > > > > > > > On Wed, Dec 02, 2015 at 06:56PM, Corneau Damien wrote: > > > > > > > I think that what moon means is that: > > > > > > > If we merge the way it is now, the KnitRInterpreter will be > part > > > of the > > > > > > > code base, so it isn't optional at code base level. > > > > > > > > > > > > > > To make it even more simple: > > > > > > > * If the code has the right licensing -> that code can be part > > of > > > the > > > > > > > source code, and can be including in a binary release > > > > > > > > > > > > We aren't concerned with binary releases - as an Apache community > > we > > > are > > > > > > voting and releasing source code. If the project wants to provide > > a > > > > > binary > > > > > > release to its users, they are better be warned about inclusion > of > > > non > > > > > > ASL2-friendly licensed bits. > > > > > > > > > > > > > * If the code doesn't have the right licensing -> it can't be > > part > > > of > > > > > the > > > > > > > source code, and can't be included in a binary release > > > > > > > > > > > > See above. > > > > > > > > > > > > > * If the code doesn't have the right licensing but is imported > > at > > > build > > > > > > > time (libraries for example) -> it is not in the source code, > it > > > can't > > > > > be > > > > > > > included in binary release > > > > > > > > > > > > That is unless a user doing it on his own. The best way to go > > around > > > it > > > > > is by > > > > > > providing a build-time option that will pull such binaries in. > But > > by > > > > > default > > > > > > such libs shouldn't be pulled. > > > > > > > > > > > > Cos > > > > > > > > > > > > > So in the case of licensing issues, it would need to be fully > > > optional > > > > > > > (user bring the interpreter in his directory and build Zeppelin > > > with > > > > > it) > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 2, 2015 at 6:33 PM, Amos B. Elberg < > > > [email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > Moon let me clarify: > > > > > > > > > > > > > > > > Interpreted code doesn’t “link.” The wiki article actually > > > explains > > > > > it > > > > > > > > pretty well — > > > https://en.wikipedia.org/wiki/GPL_linking_exception > > > > > > > > “Linking” against a library means compiling its headers into > a > > > > > binary, the > > > > > > > > way a C compiler works. The 2008 e-mail Moon distributed, > > called > > > > > this the > > > > > > > > “interpreter exception.” > > > > > > > > > > > > > > > > As for whether GPL’d code is a “mandatory dependency,” if > > knitr > > > is > > > > > missing > > > > > > > > the PR will compile, run and test just fine. The user is > never > > > > > prompted to > > > > > > > > download it. The only effect is, if the user types “knitr” > and > > > knitr > > > > > isn’t > > > > > > > > there, we display an InterpreterError that knitr isn’t there. > > > > > > > > > > > > > > > > KnitRInterpreter is not optionally required. so it does not > > > matter > > > > > KnitR > > > > > > > > is > > > > > > > > optionally required or not. > > > > > > > > Aren’t all interpreters optional? You can turn them on and > off > > > in the > > > > > > > > config files. > > > > > > > > > > > > > > > > Do you mean that the KnitRInterpreter class gets compiled to > > > > > bytecode even > > > > > > > > if knitr is missing? So what? That isn't a mandatory > > dependency > > > or a > > > > > link. > > > > > > > > > > > > > > > > From: moon soo Lee <[email protected]> > > > > > > > > Reply: [email protected] < > > > > > > > > [email protected]> > > > > > > > > Date: December 2, 2015 at 3:18:00 AM > > > > > > > > To: [email protected] < > > > > > [email protected]> > > > > > > > > Subject: Re: License of KnitRInterpreter Was: Re: > > contributions > > > > > impasse. > > > > > > > > Was: [GitHub] incubator-zeppelin pull request: R Interpreter > > for > > > > > Zeppelin > > > > > > > > > > > > > > > > Let me summarize license concern about KnitRInterpreter. > > > > > > > > > > > > > > > > > > > > > > > > Amos's interpretation. > > > > > > > > > > > > > > > > KnitR is optionally required by KnitRInterpreter. > > > > > > > > R dependency in SparkR has no problem. So KnitR should be the > > > same. > > > > > > > > > > > > > > > > > > > > > > > > Moon's interpretation. > > > > > > > > > > > > > > > > KnitRInterpreter is not optionally required. so it does not > > > matter > > > > > KnitR is > > > > > > > > optionally required or not. > > > > > > > > R dependency in SparkR is exception of GPL. KnitR is not > > applied > > > that > > > > > > > > exception. > > > > > > > > > > > > > > > > > > > > > > > > Amos, could you confirm my understanding to your > > interpretation > > > is > > > > > correct? > > > > > > > > If it is not could you clarify it? > > > > > > > > > > > > > > > > Thanks, > > > > > > > > moon > > > > > > > > > > > > > > > > On Wed, Dec 2, 2015 at 10:34 AM Amos B. Elberg < > > > > > [email protected]> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Just to put the final nail in this, I looked it up. > > > > > > > > > > > > > > > > > > I see no evidence of any “compiler exception.” > > > > > > > > > > > > > > > > > > There is an exception in the license for the runtime > > libraries > > > > > that are > > > > > > > > > bundled with GCC. See: > > > > > > > > > http://www.gnu.org/licenses/gcc-exception-3.1-faq.html > > > > > > > > > > > > > > > > > > But no “compiler exception.” > > > > > > > > > > > > > > > > > > In fact, I believe this is part of the reason why LLVM was > > > created. > > > > > > > > > > > > > > > > > > From: moon soo Lee <[email protected]> > > > > > > > > > Reply: moon soo Lee <[email protected]> > > > > > > > > > Date: December 1, 2015 at 8:16:36 PM > > > > > > > > > To: Amos B. Elberg <[email protected]>, > > > > > > > > > [email protected] < > > > > > [email protected]> > > > > > > > > > Subject: Re: contributions impasse. Was: [GitHub] > > > > > incubator-zeppelin pull > > > > > > > > > request: R Interpreter for Zeppelin > > > > > > > > > > > > > > > > > > Is knitR is commonly considered as a interpreter/compiler? > > or > > > is it > > > > > > > > > considered as a library routine? > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > moon > > > > > > > > > > > > > > > > > > On Wed, Dec 2, 2015 at 10:12 AM Amos B. Elberg < > > > > > [email protected]> > > > > > > > > > wrote: > > > > > > > > > Moon - you give this as an explanation of the licensing > > issue: > > > > > > > > > > https://stat.ethz.ch/pipermail/r-help/2008-July/169332.html > > > > > > > > > > > > > > > > > > According to that, there is an exception in the GPL for > > > interpreter > > > > > > > > > languages. As long as you don’t distribute the code, its > > fine > > > to > > > > > talk to > > > > > > > > > an interpreted language. > > > > > > > > > > > > > > > > > > Well, if that’s the case, then the PR plainly does not have > > a > > > > > license > > > > > > > > > issue. It doesn’t distribute any GPL’d R code. > > > > > > > > > > > > > > > > > > I’m not sure what’s confusing about this. It seems > > completely > > > > > > > > > straightforward. > > > > > > > > > > > > > > > > > > Regarding this: > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Amos Elberg > > > > > > > > > Sent with Airmail > > > > > > > > > > > > > > > > > > From: moon soo Lee <[email protected]> > > > > > > > > > Reply: [email protected] < > > > > > > > > > [email protected]> > > > > > > > > > Date: December 1, 2015 at 6:48:47 PM > > > > > > > > > > > > > > > > > > To: [email protected] < > > > > > [email protected] > > > > > > > > > > > > > > > > > > Subject: Re: contributions impasse. Was: [GitHub] > > > > > incubator-zeppelin pull > > > > > > > > > request: R Interpreter for Zeppelin > > > > > > > > > > > > > > > > > > On Wed, Dec 2, 2015 at 1:09 AM Amos B. Elberg < > > > > > [email protected]> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > I am going to try to minimize my reaction to Moon’s > > e-mail. > > > > > > > > > > > > > > > > > > > > The tl;dr is this: > > > > > > > > > > > > > > > > > > > > The reason we are having this discussion now is that > > active > > > > > users of > > > > > > > > the > > > > > > > > > > PR — which now has its own user base — went public to > > > complain > > > > > about > > > > > > > > > this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > The PR has been tested by an active user base for more > > than > > > three > > > > > > > > months. > > > > > > > > > > No-one has been able to identify any specific actual > > > licensing > > > > > problem, > > > > > > > > > and > > > > > > > > > > the PR was prepared based on an extensive, careful review > > of > > > the > > > > > > > > relevant > > > > > > > > > > licensing issues and after contacting the relevant > people. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I admire every software that used by user and helping > > people. > > > That > > > > > > > > includes > > > > > > > > > your work. But that's not the topic we're in discussion. > > Active > > > > > user does > > > > > > > > > not mean your contribution can ignore the review. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It is not an explanation for someone who has been > ignoring > > my > > > > > “how can > > > > > > > > I > > > > > > > > > > move this forward…” emails for three months to point the > > > finger > > > > > and > > > > > > > > say I > > > > > > > > > > didn’t contact the right person or file the right report. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This is also not the topic in this discussion. > > > > > > > > > > > > > > > > > > > > > > > > > > > > The burden for providing an explanation for the inaction > > is > > > on > > > > > the PMCC > > > > > > > > > at > > > > > > > > > > this point. > > > > > > > > > > > > > > > > > > I'm sorry, but the other PRs are passing CI. If it's > problem > > on > > > > > Zeppelin > > > > > > > > > > core, why do you think other PRs are passing CI? > > > > > > > > > > They’re not! I often see comments on PRs to just ignore > > that > > > CI > > > > > is > > > > > > > > > > failing. > > > > > > > > > > > > > > > > > > > > One of the most common reasons this PR fails CI, is CI > > > times-out > > > > > > > > > > downloading Spark to install. How could that possibly be > > > caused > > > > > by the > > > > > > > > > PR? > > > > > > > > > > > > > > > > > > > > It looks to me like the only PRs with changes to the > > relevant > > > > > parts of > > > > > > > > > the > > > > > > > > > > code — the SparkInterpreter — are being made by the > person > > > who > > > > > wrote > > > > > > > > the > > > > > > > > > > testing suite. > > > > > > > > > > > > > > > > > > > > So, that would explain why some other PRs pass CI: > Neither > > > the > > > > > > > > > > SparkInterpreter nor the testing suite are stable or > > robust, > > > but > > > > > since > > > > > > > > > the > > > > > > > > > > PRs are coming from the person who wrote both… > > > > > > > > > > > > > > > > > > > > And let's say Zeppelin core has problem and that makes > > your > > > PR > > > > > fails on > > > > > > > > > CI > > > > > > > > > > test. That's possible. But it still does not mean we can > > > merge > > > > > it with > > > > > > > > CI > > > > > > > > > > failing. > > > > > > > > > > > > > > > > > > > > It means you should be working with me to figure out why > > the > > > CI > > > > > is > > > > > > > > > failing. > > > > > > > > > > > > > > > > > > > > This PR has been tested by an active user base for the > > past > > > three > > > > > > > > months. > > > > > > > > > > If CI is continuing to fail, and dozens of hours of > effort > > > have > > > > > not > > > > > > > > > > resolved the CI issues, then it is time to start > > considering > > > > > whether > > > > > > > > the > > > > > > > > > > testing suite is part of the problem. > > > > > > > > > > > > > > > > > > > > The level of defensiveness about the CI and > > SparkInterpreter > > > is > > > > > not > > > > > > > > > > helping to resolve these issues. > > > > > > > > > > > > > > > > > > > > If you think it's problem on Zeppelin core, then file an > > > issue > > > > > that > > > > > > > > > > reproduce the problem on Zeppelin core, that might be > more > > > > > efficient > > > > > > > > than > > > > > > > > > > keep trying yourself. > > > > > > > > > > I contacted you numerous times about such issues... > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I remember i commented your issue about CI. but you just > > keep > > > > > repeated > > > > > > > > it's > > > > > > > > > not your problem but Zeppelin core problem. > > > > > > > > > > > > > > > > > > Then please file an issue about the problem you found in > > > Zeppelin > > > > > Core. > > > > > > > > > Then everyone will get into the problem. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In my interpretation, KnitRInterpreter is not an optional > > > > > feature while > > > > > > > > > it > > > > > > > > > > is always enabled when compiling Zeppelin and always > > enabled > > > when > > > > > > > > running > > > > > > > > > > Zeppelin. And it requires dynamically linked GPL library > > on > > > > > runtime. > > > > > > > > (yes > > > > > > > > > > it will fail when no KnitR is installed on the system) > > > > > > > > > > > > > > > > > > > > Its not always enabled. > > > > > > > > > > It is not dynamically linked at runtime. > > > > > > > > > > It will not fail when knitr is missing. If knitr is not > > > present, > > > > > the > > > > > > > > repl > > > > > > > > > > interpreter starts and a note is written to to the log > > that > > > the > > > > > knitr > > > > > > > > > > interpreter isn’t available because knitr is not present. > > > > > > > > > > > > > > > > > > > > no Apache code can ever call a shell script, on the > > purpose > > > of > > > > > dynamic > > > > > > > > > > linking with GPL library. > > > > > > > > > > You misunderstand. > > > > > > > > > > > > > > > > > > > > The *shell* is GPL'd. > > > > > > > > > > > > > > > > > > > > Is Zeppelin “linked" against the GPL’d shell because > > Zeppelin > > > > > depends > > > > > > > > on > > > > > > > > > a > > > > > > > > > > shell script to launch? > > > > > > > > > > > > > > > > > > > Obviously not. > > > > > > > > > > > > > > > > > > > > The interaction with R in the PR is the same. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Again, bash is one of exceptions of GPL, like other GPL > > > licensed > > > > > > > > > compiler/interpreter. > > > > > > > > > > > > > > > > > > Check here why Bash and R is okay with Apache License. > > > > > > > > > > https://stat.ethz.ch/pipermail/r-help/2008-July/169332.html > > > > > > > > > > > > > > > > > > I'm not sure we can apply the same exception for 'using' > > KnitR. > > > > > > > > > > > > > > > > > > My point is not 'KnitR' is optional or not. Point is > > > > > 'KnitRInterpreter' > > > > > > > > you > > > > > > > > > wrote is not an optional feature. Which is clearly not > > > optionally > > > > > enabled > > > > > > > > > code and feature. And that depends on KnitR library which > is > > > GPL. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I was guessing SparkR can be still in Apache License even > > if > > > it > > > > > is > > > > > > > > > depends > > > > > > > > > > on R. Because of GPL licensed compiler generated output > is > > > not > > > > > GPL > > > > > > > > > license. > > > > > > > > > > and R is sort of compiler. If you can get answer from > > Spark > > > > > community > > > > > > > > how > > > > > > > > > > SparkR get managed to stay in Apache License while R is > > GPL, > > > the > > > > > answer > > > > > > > > > > might help. > > > > > > > > > > The description of SparkR is not accurate in any respect. > > (Do > > > > > you think > > > > > > > > > > SparkR is not talking to GPL-licensed libraries?) > > > > > > > > > > > > > > > > > > > > I don’t see that any genuine issue is being raised here. > > > > > > > > > > > > > > > > > > > > If there is an issue, the burden is on you to identify > it. > > > > > > > > > > > > > > > > > > > > If i give you one suggestion, Zeppelin committers > > sometimes > > > ask > > > > > rebase > > > > > > > > > the > > > > > > > > > > contribution branch for some reason. It is not the really > > the > > > > > best > > > > > > > > > > practice, but still okay while most contributions are not > > > > > including > > > > > > > > large > > > > > > > > > > code base changes > > > > > > > > > > However, your one, has more than 4000 lines of code > > change. > > > If > > > > > you > > > > > > > > rebase > > > > > > > > > > then review should be started from the beginning, again. > > So > > > you > > > > > might > > > > > > > > > want > > > > > > > > > > to minimize the rebase your branch. > > > > > > > > > > > > > > > > > > > > Are you actually complaining that the problem is that I > > > rebased > > > > > the > > > > > > > > code > > > > > > > > > > during the three-month period when no-one looked at it > and > > > > > Zeppelin > > > > > > > > went > > > > > > > > > > through a release? > > > > > > > > > > > > > > > > > > > > I cannot take it seriously when you say things like this. > > > > > > > > > > > > > > > > > > > > Having to “start from the beginning” cannot be a problem > > if > > > you > > > > > never > > > > > > > > > > actually started the first time... > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You wanted coordination and cooperation. So i gave you > > > suggestion > > > > > that > > > > > > > > > helping review process. For example, your code has been > > rebased > > > > > since my > > > > > > > > > comment and jongyoul's comment. that means committers will > > > need to > > > > > look > > > > > > > > > from the beginning. That'll require more time. And maybe, i > > > guess > > > > > that's > > > > > > > > > not what you want. But If you don't care, feel free to > > rebase. > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > moon > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: moon soo Lee <[email protected]> > > > > > > > > > > Reply: moon soo Lee <[email protected]> > > > > > > > > > > Date: December 1, 2015 at 4:42:06 AM > > > > > > > > > > To: Amos B. Elberg <[email protected]>, > > > > > > > > > > [email protected] < > > > > > [email protected]> > > > > > > > > > > Subject: Re: contributions impasse. Was: [GitHub] > > > > > incubator-zeppelin > > > > > > > > pull > > > > > > > > > > request: R Interpreter for Zeppelin > > > > > > > > > > > > > > > > > > > > On Tue, Dec 1, 2015 at 4:40 PM Amos B. Elberg < > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > Thank you, Cos. > > > > > > > > > > > > > > > > > > > > I’d like to briefly address the issues raised by Moon: > > > > > > > > > > > > > > > > > > > > 1. This PR does not passes CI > > > > > > > > > > The CI fails on core Zeppelin, *not* code in this PR. > > > > > > > > > > > > > > > > > > > > I’ve been seeking assistance on this since August. > > > > > > > > > > > > > > > > > > > > The most common reason is that SparkInterpreter is unable > > to > > > > > launch > > > > > > > > Spark > > > > > > > > > > and open a Spark Backend. This is necessary to test the > > PR. > > > > > > > > > > > > > > > > > > > > 60+ hours, has been spent adapting and re-basing when the > > > > > > > > > SparkInterpreter > > > > > > > > > > architecture changed and broke the PR’s *tests.* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm sorry, but the other PRs are passing CI. If it's > > problem > > > on > > > > > > > > Zeppelin > > > > > > > > > > core, why do you think other PRs are passing CI? > > > > > > > > > > > > > > > > > > > > And let's say Zeppelin core has problem and that makes > > your > > > PR > > > > > fails on > > > > > > > > > CI > > > > > > > > > > test. That's possible. But it still does not mean we can > > > merge > > > > > it with > > > > > > > > CI > > > > > > > > > > failing. > > > > > > > > > > > > > > > > > > > > If you think it's problem on Zeppelin core, then file an > > > issue > > > > > that > > > > > > > > > > reproduce the problem on Zeppelin core, that might be > more > > > > > efficient > > > > > > > > than > > > > > > > > > > keep trying yourself. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Not 100% sure this PR has no license issue. (about > > KniteR) > > > > > > > > > > What license problem *specifically* do you believe may > > exist? > > > > > > > > > > > > > > > > > > > > In preparing the PR, I: > > > > > > > > > > > > > > > > > > > > * Reviewed the Apache policy in detail. > > > > > > > > > > > > > > > > > > > > * Contacted the FSF to ask their interpretation of the > > > “linking” > > > > > > > > > > provisions of the GPL license. > > > > > > > > > > > > > > > > > > > > * Reviewed how other Apache software deals with this > issue > > > > > (e.g., Spark > > > > > > > > > > talks to R, which is GPL'd). > > > > > > > > > > > > > > > > > > > > * No necessary *dependencies* of the PR have license > > > conflicts. > > > > > In > > > > > > > > > > several cases, I contacted package authors who agreed to > > > > > re-issue their > > > > > > > > > > packages under Apache-compatible licenses. (Usually I had > > to > > > do > > > > > a bit > > > > > > > > of > > > > > > > > > > coding in exchange…) > > > > > > > > > > > > > > > > > > > > * Where the license had to stay GPL, the packages are > *not > > > > > necessary* > > > > > > > > and > > > > > > > > > > *not dependencies.* If the optional packages are present, > > > they > > > > > will be > > > > > > > > > > used to enable additional functionality. Knitr is an > > example. > > > > > The PR > > > > > > > > will > > > > > > > > > > compile and run fine without knitr. If knitr is available > > > (it is > > > > > part > > > > > > > > of > > > > > > > > > > the most common R distribution), the PR will enable the > > knitr > > > > > > > > > interpreter. > > > > > > > > > > > > > > > > > > > > * This is exactly how this issue is addressed through the > > > Apache > > > > > > > > > > ecosystem. > > > > > > > > > > The tl;dr is this: When Apache code is written to talk to > > > > > libraries > > > > > > > > that > > > > > > > > > > may or may not be present on the user’s system, or where > > it > > > > > talks to an > > > > > > > > > API > > > > > > > > > > but is agnostic about implementation, that is not > > “linking” > > > in a > > > > > way > > > > > > > > that > > > > > > > > > > implicate the anti-linking provision of the GPL. > > > > > > > > > > > > > > > > > > > > Otherwise, no Apache code could ever call a shell script! > > Let > > > > > alone run > > > > > > > > > > on Linux, or talk to R. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm not a legal expert. So following could be wrong. > > > > > > > > > > > > > > > > > > > > In my interpretation, KnitRInterpreter is not an optional > > > > > feature while > > > > > > > > > it > > > > > > > > > > is always enabled when compiling Zeppelin and always > > enabled > > > when > > > > > > > > running > > > > > > > > > > Zeppelin. And it requires dynamically linked GPL library > > on > > > > > runtime. > > > > > > > > (yes > > > > > > > > > > it will fail when no KnitR is installed on the system) > > > > > > > > > > > > > > > > > > > > And of course, no Apache code can ever call a shell > > script, > > > on > > > > > the > > > > > > > > > purpose > > > > > > > > > > of dynamic linking with GPL library. > > > > > > > > > > > > > > > > > > > > I was guessing SparkR can be still in Apache License even > > if > > > it > > > > > is > > > > > > > > > depends > > > > > > > > > > on R. Because of GPL licensed compiler generated output > is > > > not > > > > > GPL > > > > > > > > > license. > > > > > > > > > > and R is sort of compiler. > > > > > > > > > > > > > > > > > > > > If you can get answer from Spark community how SparkR get > > > > > managed to > > > > > > > > stay > > > > > > > > > > in Apache License while R is GPL, the answer might help. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. Need more time to review. > > > > > > > > > > Has any reviewer has downloaded the PR or run the demo > > > notebook? > > > > > (Which > > > > > > > > > > is there for the benefit of reviewers, and isn’t intended > > to > > > go > > > > > in > > > > > > > > final > > > > > > > > > > distribution.) > > > > > > > > > > > > > > > > > > > > How many +1 comments from users would you like to see? > > > > > > > > > > > > > > > > > > > > How much time do you believe is required? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It all depends on when CI is going to pass, when license > > > problem > > > > > is > > > > > > > > going > > > > > > > > > > to be cleared, and when a committer willing to review and > > > > > responsible > > > > > > > > to > > > > > > > > > > commit your contribution. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. Large code base change > > > > > > > > > > Large code base changes require coordination and > > cooperation. > > > > > This PR > > > > > > > > > > necessarily implicates the build scripts, testing code, > > the > > > > > > > > > > SparkInterpreter, etc. > > > > > > > > > > > > > > > > > > > > I have been seeking to coordinate since August. > > > > > > > > > > > > > > > > > > > > I continue to stand ready to do so. > > > > > > > > > > > > > > > > > > > > -Amos > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > If i give you one suggestion, Zeppelin committers > > sometimes > > > ask > > > > > rebase > > > > > > > > > the > > > > > > > > > > contribution branch for some reason. It is not the really > > the > > > > > best > > > > > > > > > > practice, but still okay while most contributions are not > > > > > including > > > > > > > > large > > > > > > > > > > code base changes. > > > > > > > > > > > > > > > > > > > > However, your one, has more than 4000 lines of code > > change. > > > If > > > > > you > > > > > > > > rebase > > > > > > > > > > then review should be started from the beginning, again. > > So > > > you > > > > > might > > > > > > > > > want > > > > > > > > > > to minimize the rebase your branch. > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > moon > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: moon soo Lee <[email protected]> > > > > > > > > > > Reply: [email protected] < > > > > > > > > > > [email protected]> > > > > > > > > > > Date: December 1, 2015 at 1:34:19 AM > > > > > > > > > > To: [email protected] < > > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > > Subject: Re: contributions impasse. Was: [GitHub] > > > > > incubator-zeppelin > > > > > > > > pull > > > > > > > > > > request: R Interpreter for Zeppelin > > > > > > > > > > > > > > > > > > > > Hi Cos, > > > > > > > > > > > > > > > > > > > > Thanks for opening a discussion. > > > > > > > > > > My answer about 'Why this PR is open for three months' is > > > > > > > > > > > > > > > > > > > > 1. This PR does not passes CI > > > > > > > > > > 2. Not 100% sure this PR has no license issue. (about > > KniteR) > > > > > > > > > > 3. Need more time to review. > > > > > > > > > > > > > > > > > > > > It's my personal answer, other committers may have > > different > > > > > opinion. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think the question should be generalized. Because this > > PR > > > is > > > > > not the > > > > > > > > > only > > > > > > > > > > PR that is in impasse. There're more. For example > > > > > > > > > > > > > > > > > > > > Here's some examples that PR is not been merged. > > > > > > > > > > https://github.com/apache/incubator-zeppelin/pull/53, > > > > > > > > > > https://github.com/apache/incubator-zeppelin/pull/60 > > > > > > > > > > and so on. > > > > > > > > > > > > > > > > > > > > I can categorize the cases, based on experience of > > involving > > > > > Zeppelin > > > > > > > > > > community from the beginning. > > > > > > > > > > > > > > > > > > > > 1. Large code base change > > > > > > > > > > > > > > > > > > > > When contribution has large code base changes, it tend to > > > take > > > > > more > > > > > > > > time > > > > > > > > > to > > > > > > > > > > review and merged. Normally, most contributions merged in > > > 1day~1 > > > > > week. > > > > > > > > > > But some contribution has large code base changes take > 2~4 > > > > > weeks. Few > > > > > > > > > > contribution that has very large code base change take > > > months. > > > > > > > > > > > > > > > > > > > > 2. Communication lost > > > > > > > > > > > > > > > > > > > > Sometimes, committer is not responding, or contributor is > > not > > > > > > > > responding. > > > > > > > > > > > > > > > > > > > > 3. Opinion diverges > > > > > > > > > > > > > > > > > > > > For some changes, included in contribution. When people > > have > > > > > different > > > > > > > > > > opinion and it continue to diverges, those PRs are not > > been > > > > > merged. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think having a guide such as ping other committer when > a > > > > > committer is > > > > > > > > > not > > > > > > > > > > responding, and divide contribution into small peaces if > > > > > possible, > > > > > > > > would > > > > > > > > > > help most of the cases. > > > > > > > > > > Of course committer have to pay attention more to the > > > > > contribution and > > > > > > > > > > helping people. That's the first one. > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > moon > > > > > > > > > > > > > > > > > > > > On Tue, Dec 1, 2015 at 1:54 PM Konstantin Boudnik < > > > > > [email protected]> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > To make sure we're on the same page, here are two > > threads > > > that > > > > > I > > > > > > > > found > > > > > > > > > > > related > > > > > > > > > > > to this topic. > > > > > > > > > > > > > > > > > > > > > > Thread 1: > > > > > > > > > > > Subject: R? > > > > > > > > > > > Started on: July 1, 2015 > > > > > > > > > > > > > > > > > > > > > > Thread 2: > > > > > > > > > > > Subject: [GitHub] incubator-zeppelin pull request: R > > > > > Interpreter for > > > > > > > > > > > Zeppelin > > > > > > > > > > > Started on: August 13, 2015 > > > > > > > > > > > > > > > > > > > > > > If you want to fetch these from our archive send emails > > to > > > > > > > > > > > [email protected] > > > > > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > > > > Cos > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 30, 2015 at 06:27PM, Konstantin Boudnik > > wrote: > > > > > > > > > > > > Guys, > > > > > > > > > > > > > > > > > > > > > > > > While catching up on my emails from the last a couple > > of > > > > > weeks, > > > > > > > > this > > > > > > > > > > > thread > > > > > > > > > > > > caught my attention. I am not normally paying much > > > attention > > > > > to the > > > > > > > > > > code > > > > > > > > > > > > reviews traffic from GH, but this one is pretty > > > different as > > > > > it > > > > > > > > spans > > > > > > > > > > > three > > > > > > > > > > > > months and counting. > > > > > > > > > > > > > > > > > > > > > > > > First, here are my five cents: > > > > > > > > > > > > - r/R/rzeppelin/LICENSE is wrong: if the code is > aimed > > > to be > > > > > > > > > > > contributed to > > > > > > > > > > > > an ASF project this file should simply contain ASL2 > > text, > > > > > like in > > > > > > > > [1] > > > > > > > > > > > > - r/pom.xml perhaps shouldn't contain a separate > > > <developers> > > > > > > > > > section, > > > > > > > > > > > but > > > > > > > > > > > > Zeppelin might have different guidelines on it. Say, > > > Bigtop > > > > > doesn't > > > > > > > > > > > > maintain this in the source code, but we have the > list > > of > > > > > all the > > > > > > > > > > > > committers on the project's site [2] Every new > > > committer's > > > > > first > > > > > > > > > > > commit is > > > > > > > > > > > > to update the team page ;) > > > > > > > > > > > > - comments like in > > > > > > > > > > > > > r/src/main/java/org/apache/zeppelin/rinterpreter/KnitR.java > > > > > > > > > > > > > > > > > > > > > > > > +/** > > > > > > > > > > > > + * Created by aelberg on 7/28/15. > > > > > > > > > > > > + */ > > > > > > > > > > > > > > > > > > > > > > > > is better to be removed. It has been already > discussed > > > here > > > > > [3]. > > > > > > > > And > > > > > > > > > > > the > > > > > > > > > > > > initial discussion on the topic [4] was linked as > well > > > > > > > > > > > > - same goes to r/R/rzeppelin/DESCRIPTION. I am not > > sure > > > if > > > > > this is > > > > > > > > > > > R-specific > > > > > > > > > > > > stuff - I have no idea about R, honestly, but > > > > > > > > > > > > > > > > > > > > > > > > +License: GPL (>= 2) | BSD_3_clause + file LICENSE > > > > > > > > > > > > ... > > > > > > > > > > > > +Author: David B. Dahl > > > > > > > > > > > > > > > > > > > > > > > > shouldn't be here, IMO. Normally, if some additional > > > > > licenses are > > > > > > > > > > > used, > > > > > > > > > > > > they have to be listed in the top-level NOTICE file > > > (already > > > > > > > > there). > > > > > > > > > > > > > > > > > > > > > > > > - I am not going to offer any comments on the > > technical > > > > > merits of > > > > > > > > the > > > > > > > > > > > patch, > > > > > > > > > > > > as I haven't tried it personally. However, I don't > see > > > any > > > > > serious > > > > > > > > > > > > technical objections to the functionality in > question. > > > > > > > > > > > > > > > > > > > > > > > > So, the question is - why the PR is open for three > > > months? I > > > > > hasn't > > > > > > > > > > been > > > > > > > > > > > able > > > > > > > > > > > > to get a clear answer. What I found out though is > > pretty > > > > > > > > unsettling, > > > > > > > > > > > really. > > > > > > > > > > > > The communication on the topic is almost > non-existent, > > > > > except for > > > > > > > > > this > > > > > > > > > > > sparse > > > > > > > > > > > > and bitter thread in the GH. > > > > > > > > > > > > > > > > > > > > > > > > I would love to hear from the committers what's > > stopping > > > the > > > > > > > > > acceptance > > > > > > > > > > > of the > > > > > > > > > > > > code, besides of the minor issues I've mentioned > > earlier? > > > > > What are > > > > > > > > > the > > > > > > > > > > > reasons for it? > > > > > > > > > > > > Is there anything wrong with it? > > > > > > > > > > > > One of the responsibilities of the committers is to > > make > > > > > sure the > > > > > > > > > > > > contributions are reviewed; new contributors are > > guided > > > and > > > > > do > > > > > > > > > > > understand how > > > > > > > > > > > > the project ticks. The easy feedback flow attracts > new > > > > > people, > > > > > > > > > allowing > > > > > > > > > > > the > > > > > > > > > > > > community to grow and thrive. > > > > > > > > > > > > > > > > > > > > > > > > I am asking _explicitely_ not to re-start the > > bickering I > > > > > have > > > > > > > > > already > > > > > > > > > > > > seen. At this point I am interested in the purely > > > technical > > > > > side of > > > > > > > > > > this. > > > > > > > > > > > > > > > > > > > > > > > > [1] > > https://github.com/apache/bigtop/blob/master/LICENSE > > > > > > > > > > > > [2] http://bigtop.apache.org/team-list.html > > > > > > > > > > > > [3] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://apache-nifi-developer-list.39713.n7.nabble.com/author-tags-td1335.html > > > > > > > > > > > > [4] http://s.apache.org/iZl > > > > > > > > > > > > > > > > > > > > > > > > With regards, > > > > > > > > > > > > Cos > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 16, 2015 at 11:06PM, elbamos wrote: > > > > > > > > > > > > > Github user elbamos commented on the pull request: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-157203411 > > > > > > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
