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. > > > > > > > > > > > > > > > > > > > > > > > >
signature.asc
Description: Digital signature
