Currently, we agree on the docker R engine init support for sure and I think no one is against it.
I don't have a specific opinion about the naming part, but I do believe we need more modifications for those filenames and package names if we decided to change it. +0 for naming change I don't think we need to vote on the other parts (docker R init) either. but I can give my +1 on those if counted. Let's quickly go through the process and push the project forward. Best Regards Wei On Fri, May 24, 2019 at 8:27 AM Daniel Takabayashi < daniel.takabaya...@gmail.com> wrote: > Lucas, to avoid conflicts the Apache rules allow us to call a vote session. > > https://httpd.apache.org/dev/guidelines.html > > Please just vote. > > Thanks, > Taka > > Sent from my iPhone > > > On May 23, 2019, at 1:40 PM, Lucas Bonatto Miguel < > lucasbona...@gmail.com> wrote: > > > > Daniel the only thing we're changing is that I am reverting the MR, and > > creating a new MR for the engine-server renaming changes. So that we can > > give more tests to it. > > > > We do have a docker image that supports running Marvin and R right now. > > There is no way I can even test an R engine while we don't have an R > common > > lib, which is part of https://issues.apache.org/jira/browse/MARVIN-3 . > This > > refactoring is being done in parts, not all at once. > > > > All the other items I believe are waste of time discussing, given my MR > was > > standing for 2 weeks and got approved recently. > > > > If you figured out a better way of doing MARVIN-1, please send us an MR > and > > we can discuss that. > > > > Regards, > > Lucas > > > > On Thu, May 23, 2019 at 1:19 PM Daniel Takabayashi < > > daniel.takabaya...@gmail.com> wrote: > > > >> So, guys, we have three actions over here, and I think we should vote > ASAP > >> to avoid uselessly efforts. > >> > >> 1 - Revert the PR#21 > >> 2 - Change the name from engine-executor to engine-server > >> 3 - New PR to allow us to test an review of the new R support. > >> > >> +1 for number 1 > >> Justification: Because I believe is the best way to avoid future > problems > >> and to give us a chance to do a better code review > >> > >> -1 for number 2 > >> Justification: I don't see any reason to do this, also the efforts to > >> change, test and validate everything are huge. Besides the fact that we > >> going to lose a lot of external references from articles, presentations, > >> manuals and etc. > >> > >> +1 for number 3 but > >> Justification: The only way this makes sense for me it is following the > >> decisions we have made, using as reference the architecture designed > before > >> (link in my previous email). Following this rationale, what Lucas was > >> trying to merge should be this component: > >> > >> *item 5 - Create an empty engine template (r-engine-template) to be > used to > >> create new engines.* > >> > >> > >> Sorry guys to be so restricted in this case asking to follow the > previous > >> design but once there is a bunch of new pieces around this new feature. > Not > >> respecting these rules will make very hard for us to archive our goals > once > >> the communication will change after each newly merged code. And Lucas, > I am > >> sorry it is not about your code I think the misunderstanding here was > about > >> the task at self. and how long took for us to start to really review > your > >> PR. > >> > >> Em qui, 23 de mai de 2019 às 12:36, Lucas Bonatto Miguel < > >> lucasbona...@gmail.com> escreveu: > >> > >>> Sounds good, I'll do that and I send the new PR link here soon. > >>> > >>> Best, > >>> Lucas > >>> > >>>> On Thu, May 23, 2019 at 12:33 PM Wei Chen <weic...@apache.org> wrote: > >>>> > >>>> One thing that I noticed is that: > >>>> If we go into the code and check the files. > >>>> They are still "executor". ex: EngineExecutorApp.scala > >>>> If we are going to change "executor" to "server", > >>>> we might need to modify the packages and most files. > >>>> > >>>> I think it will be simpler for us to revert PR#21 first. > >>>> Only make changes to include docker R support. (without changing name) > >>>> Make another PR about name changing and we can discuss there. > >>>> > >>>> Best Regards > >>>> Wei > >>>> > >>>> On Thu, May 23, 2019 at 1:21 PM Lucas Bonatto Miguel < > >>> lucasb...@apache.org > >>>>> > >>>> wrote: > >>>> > >>>>> Hey guys, in my last MR ( > >>>>> https://github.com/apache/incubator-marvin/pull/21) > >>>>> I added support for building docker images and also started > >>> implementing > >>>>> what we discussed as being the new architecture, that will have its > >>> roots > >>>>> on containers and REPL. > >>>>> > >>>>> During the work I realized that in the new context the > >> engine-executor > >>>>> would look more like an engine-server, given it's, in fact, a server > >>> for > >>>>> marvin engines implemented in any language. > >>>>> > >>>>> I think I underestimated the impact of this change, and now I > >> realized > >>> I > >>>>> will need to send some updates on the documentation. Other than that, > >>>> does > >>>>> anyone see any other thing that needs to be done? > >>>>> > >>>>> Best, > >>>>> Lucas > >>>>> > >>>> > >>> > >> >
