Moon - no. That is the opposite of what people are saying. I started this thread because I feel that you are disregarding the consensus of the community.
The thread asks for two things - 208 to be merged without further delay, and for you to stay out of the issue of R interpreters entirely. 702 can be addressed after 208 is merged. How many more people do you need to hear from? > On Mar 29, 2016, at 5:40 AM, moon soo Lee <m...@apache.org> wrote: > > Hi folks, > > I didn't see anyone disagreement merge 208 and/or 702 in this thread and > previous thread [1], as they're ready. while they both have technical > merits as Jeff summarized really well. > > Now i can see 208 finally made some progress on CI [2]. Hope continue the > work and make CI green. Also I can see 702 is trying to finishing up and > waiting for CI become green. > > I don't want to merge something that breaks CI. If then, it becomes make > very difficult to verify all other contributions. Other contributions are > as important as these two. Hope community can understand that. > > Considering recent progress of both contributions, i expect they'll be > ready anytime soon. And then we can finally merge them. > > About merging 702, 208 contributions, does this sounds clear? > > If they're both merged, It's possible to improve both RInterpreter by > taking each others advantage. Therefore, no reason to worry at this point > about which one is better, which one has advantages, which one will merge > before the other, etc. Both have pros and cons and both will help Zeppelin > thankfully. > > Thanks, > moon > > [1] > http://apache-zeppelin-incubating-dev-mailing-list.75694.x6.nabble.com/R-interpreter-in-Zeppelin-further-steps-tp6967.html > [2] > https://github.com/apache/incubator-zeppelin/pull/208#issuecomment-202682652 > > >> On Tue, Mar 29, 2016 at 1:45 AM enzo <e...@smartinsightsfromdata.com> wrote: >> >> I am looking forward to see 208 merged, *soon* please. From my tests it >> seems that this should be the priority. >> >> I think 702 has merits (but I’ve used it less) and deserves to be merged >> too once ready. >> >> Ultimately after a period of "real road” testing we will be able to >> understand what we really need. >> >> E.g. from past discussions I am not convinced that either PR would, >> as-it-is, support fully the needs of a multi-user Zeppelin Server approach >> (something similar to RStudio Server Professional to get an idea). A >> period of use and gradual evolution (and possibly merge?) will be required. >> >> The sooner we start the better. >> >> >> >> Enzo >> e...@smartinsightsfromdata.com >> >> >> >>>> On 29 Mar 2016, at 07:08, Jeff Steinmetz <jeffrey.steinm...@gmail.com> >>> wrote: >>> >>> I’m not affiliated to either author nor have any attachment to an >> specific outcome - and happy to continue being as objective and unbiased as >> possible. >>> >>> >>> I would say they now have different philosophical approaches (as of the >> March 23rd merge of datalayer#7 to 702). >>> I agree with Amos Elberg that 702 has changed directions a few times. >>> >>> Re: commits to 702 by Leemoonsoo on March 23 via datalayer#7: >>> I found the current state of the 702 PR to be succinct, in terms of >> it’s code base, via its use of the SparkR dependency - which is already >> baked into spark distribution. >>> >>> The 702 code base appears to be easier to maintain using this approach >> (less code, no rscala source, no BSD licensing additions required, easier >> to read). >>> 702 packages correctly with -Pbuild-distr as expected - i.e. it works >> out of gate from the distribution directory. >>> The build profile -Psparkr worked as expected, and the addition of this >> profile felt intuitive to me. >>> >>> >>> Myself and a colleague that uses R extensively noticed (as Amos Elberg >> reminded us): >>> 208 handles passing arrays and other data types between scala & R more >> gracefully than 702. >>> 208 handles the output of intermediate R calls more gracefully than 702. >>> >>> Beyond that: >>> 208 Requires SPARK_HOME to be set or the interpreter hangs with no >> error. It’s been mentioned by the 208 author that the requirement to set >> SPARK_HOME is a feature. I think we could revisit this assumption now that >> I see how 702 handles this with defaults via a graceful fallback. >>> 702 works fine with zero configuration, I.e for those that want to test >> locally with no separate spark distribution installed (SPARK_HOME does not >> need to be set). >>> 702 having just an %r interpreter, and having it as part of the spark >> interpreter (same subdirectory) feels like a cleaner approach (this is >> arguably a philosophical difference again). >>> >>> >>> It feels like an exhaustive list of `.z.show.googleVis(Motion)` type >> calls in 208 could bloom into unnecessary code maintenance overhead and >> required additions every time a new chart library is introduced, vs. a more >> generic show method. Perhaps a follow on improvement post merge (applies >> to both PRs). >>> This same chart rendering works in 702 with `print(Motion, tag='chart’)` >> which isn’t necessarily better or worse, again, a different philosophical >> approach. >>> >>> They both have merit in different regards. It’s doesn’t feel >> appropriate to make a broad statement that "no-one supported 702”. >>> If I had a magic wand, it would be a hybrid of the two approaches. >>> >>> I look forward to continuing the review of each PR individually or both >> collaboratively. >>> >>> Regards, >>> Jeff >>> >>> >>>> On 3/28/16, 4:13 PM, "Sourav Mazumder" <sourav.mazumde...@gmail.com> >>> wrote: >>> >>>> All said and done we had enough discussion on this point for many months >>>> now. As far as I know, 208 is the PR which community/people have so far >>>> used mostly and successfully (at least me and whoever I introduced to >> 208 >>>> for SparkR support). I thought it was going to be merged a long time >> ago. >>>> May be what will make sense is to first integrate the 208. After that, >> a >>>> new PR can be created on that and if 702 has anything extra then that >>>> feature can be added. >>>> >>>> Regards, >>>> Sourav >>>> >>>> >>>> On Mon, Mar 28, 2016 at 12:37 AM, Eran Witkon <eranwit...@gmail.com> >> wrote: >>>> >>>>> @Elberg, If I were you I would ask myself why isn't the community >> taking >>>>> part in this debate? >>>>> Personally I prefer a team player as a contributor over the best >> developer. >>>>> just my 2c >>>>> Eran >>>>> >>>>> On Mon, 28 Mar 2016 at 09:52 Amos B. Elberg <amos.elb...@gmail.com> >> wrote: >>>>> >>>>>> Moon - I opened this discussion so it could take place with the >> community >>>>>> as a whole, not just you. >>>>>> >>>>>> Suffice it to say, I disagree with every one of the technical claims >>>>>> you've just made, and I don't trust your intent. >>>>>> >>>>>> Let the community process happen. >>>>>> >>>>>>> On Mar 28, 2016, at 2:47 AM, moon soo Lee <m...@apache.org> wrote: >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Simply put, >>>>>>> >>>>>>> - 702 and/or 208 will can merged as they're ready. [1] >>>>>>> - 208 will not be merged while it does not pass CI. If you think code >>>>> in >>>>>>> 208 is not a problem but CI itself or other part of Zeppelin is >>>>> problem, >>>>>>> then that particular problem be fixed before merge 208. >>>>>>> - 702 has proper integration test [2] >>>>>>> >>>>>>> I'm not sure why you're so hard at devaluating 702. >>>>>>> 702 is not something you need to beat and win. 702 is something you >>>>> need >>>>>> to >>>>>>> help / learn / collaborate. >>>>>>> >>>>>>> Will you able to show your ability to collaborate with other >> community >>>>>>> members? >>>>>>> >>>>>>> Thanks, >>>>>>> moon >>>>>>> >>>>>>> [1] >> http://apache-zeppelin-incubating-dev-mailing-list.75694.x6.nabble.com/R-interpreter-in-Zeppelin-further-steps-tp6967.html >>>>>>> [2] >> https://github.com/apache/incubator-zeppelin/pull/702/files#diff-64a9440e811c5fba6ac1b61157fa6912R87 >>>>>>> >>>>>>> >>>>>>>> On Sun, Mar 27, 2016 at 7:11 PM Amos Elberg <amos.elb...@gmail.com> >>>>>> wrote: >>>>>>>> >>>>>>>> I am saddened to have to start this thread *again*. While I thought >>>>> we >>>>>> had >>>>>>>> reached consensus on this, several times over, apparently some >> people >>>>>>>> disagree. I hope this will be the last time. >>>>>>>> >>>>>>>> With this thread, I am asking the community to reach consensus (1) >>>>> That >>>>>> 208 >>>>>>>> should be merged this week, without further delay; and (2) That Moon >>>>> Lee >>>>>>>> Soo and Felix Cheung take no further part in the discussions of 208 >>>>> and >>>>>>>> 702. >>>>>>>> >>>>>>>> This PR has been pending since August. It has been stalled that >> entire >>>>>> time >>>>>>>> for no technical reason. >>>>>>>> >>>>>>>> We reached agreement to merge 208 in November, again in December, >> and >>>>>> again >>>>>>>> in February -- when Moon agreed to stay out of the issue. At that >>>>>> point, >>>>>>>> Alex, I, and others, began working on it, and appeared to be making >>>>>>>> substantial progress. >>>>>>>> >>>>>>>> And then Alex just stopped. Instead, he commenced the thread saying >>>>>> that a >>>>>>>> consensus had to be reached on 208 and 702. Until that point, >>>>>> essentially >>>>>>>> no-one had paid attention to 702. In the discussion that followed, >> we >>>>>>>> reached a consensus to merge 208 as soon as possible. After the >>>>> thread >>>>>> had >>>>>>>> died, Alex asked if anyone had additional comments, and Moon >> popped-in >>>>>> to >>>>>>>> insist that both PRs be merged. Again, no-one supported 702. At >> all. >>>>>>>> >>>>>>>> Each time I said "we had a consensus before, does anyone want to >>>>> change >>>>>>>> it," Alex or Moon steered the discussion away. The final vote was >> not >>>>>> to >>>>>>>> merge 702 or merge "both" -- it was to treat them as normal PRs. >>>>>> (Although >>>>>>>> one person did want both merged simultaneously.) That would mean >>>>>>>> completing 208 on its merits and then evaluating 702. >>>>>>>> >>>>>>>> At the time, I objected to the discussion, because I thought the >> whole >>>>>>>> thing was a contrived excuse for Moon to reject 208 by pushing 702. >>>>>> That >>>>>>>> is exactly what he is now seeking to do. >>>>>>>> >>>>>>>> *Status of 208 & 702* >>>>>>>> >>>>>>>> PR 208 has been feature-complete and testable since early September. >>>>> It >>>>>>>> has been adopted by more than 1000 users, who I have been supporting >>>>> for >>>>>>>> more than six months. The code has not undergone any major changes >>>>>> since >>>>>>>> September. There are no known bugs, and no outstanding feature >>>>> requests >>>>>>>> that can be satisfied without major changes to the Zeppelin >>>>>> architecture. >>>>>>>> >>>>>>>> 208 does *not* fail CI. 208 includes extensive unit tests of the >>>>>> R-Spark >>>>>>>> integration because this turned out to get broken by changes in >>>>> Zeppelin >>>>>>>> often. Because CI is unable at present to provide a consistent >>>>>>>> environment, 208's *OWN UNIT TESTS*, which pass when run on an >>>>> ordinary >>>>>>>> machine, fail when run on CI. >>>>>>>> >>>>>>>> 208 does need a push for compatibility with a recently adopted PR -- >>>>>> that >>>>>>>> is work I've essentially completed, but have not pushed. >>>>>>>> >>>>>>>> PR 702 is a re-design based on 208 -- not just architecture, but >> right >>>>>> down >>>>>>>> to the choice of demo images, which were taken from 208's >>>>> documentation. >>>>>>>> In fact, 702 has had been re-engineered several times to more >> closely >>>>>>>> conform to 208's architecture and feature set. But 702 still >> remains >>>>>>>> feature-incomplete -- it cannot handle the range of visualizations, >> R >>>>>>>> classes, etc., that 208 can. It is not stable code, and shows no >> signs >>>>>> of >>>>>>>> stabilizing any time soon. >>>>>>>> >>>>>>>> No-one has adopted 702. It has changed radically, fundamentally, at >>>>>> least >>>>>>>> 4 times over the past two months since it was submitted. One of >> those >>>>>>>> changes was only days ago. >>>>>>>> >>>>>>>> 702 also has no proper tests, which is the excuse for not merging >> 208. >>>>>> 702 >>>>>>>> has things labelled "tests," but they don't actually attempt to >>>>> connect >>>>>> to >>>>>>>> R or Spark, which are the things that break and which therefore need >>>>>>>> testing. >>>>>>>> >>>>>>>> *** >>>>>>>> >>>>>>>> I would like credit for my own work and design. I think I have more >>>>> than >>>>>>>> earned that. >> >>
