Val, Together with Semyon Danilov I did final polishing of code and merged it to the main branch in ignite-3 repo.
Code assembles without any issues, tests are green, IgniteRunner starts and serves REST requests successfully. On Tue, Dec 15, 2020 at 10:37 PM Valentin Kulichenko < valentin.kuliche...@gmail.com> wrote: > Thanks, Sergey! Looks good to me. > > -Val > > On Tue, Dec 15, 2020 at 12:12 AM Sergey Chugunov < > sergey.chugu...@gmail.com> > wrote: > > > Val, > > > > Your comments make total sense to me, I've fixed them and updated pull > > request. Please take a look at my code when you have time. > > > > I also added a port range configuration to enable starting multiple > > instances of ignite without specifying port manually for each instance. > > > > -- > > Best Regards, > > Sergey Chugunov > > > > On Sat, Dec 12, 2020 at 3:20 AM Valentin Kulichenko < > > valentin.kuliche...@gmail.com> wrote: > > > > > Hi Sergey, > > > > > > Thanks for doing this. > > > > > > It looks like PR #5 is already under review, so I guess it will be > merged > > > soon. I would really love to see that, because the configuration > > framework > > > is one of the foundational components - we need it to continue building > > > Ignite 3.0. > > > > > > As for PR #6, it looks a little raw, but I believe we need it to > connect > > > the configuration framework with the CLI tool that is also pending for > > the > > > merge, is this correct? If that's the case, I think it's OK to merge > this > > > code as a separate module, with an understanding that it will change > > > significantly down the road. I would do a couple of changes though: > > > > > > 1. Get rid of "simplistic-ignite" naming, as it's a little > confusing. > > > Even though it's more of a prototype at this point, it should be > clear > > > what > > > the module is responsible for. Can we rename it to "ignite-runner" > or > > > something along those lines? > > > 2. Update the output - I don't like that it prints out the > > > Javalin's banner and messages. I suggest replacing this with some > very > > > basic Ignite logging: an entry showing the version of Ignite; an > entry > > > indicating that the REST protocol is enabled on a certain port; an > > entry > > > that the process is successfully started. This is just to make sure > > that > > > anyone who plays with it understands what's going on. > > > > > > Any objections? > > > > > > -Val > > > > > > On Fri, Dec 11, 2020 at 9:53 AM Sergey Chugunov < > > sergey.chugu...@gmail.com > > > > > > > wrote: > > > > > > > Hello Igniters, > > > > > > > > I would like to present two pull requests [1], [2] with basic > > > > implementation of IEP-55 for Unified Configuration [3] and IEP-63 > REST > > > API > > > > for Unified Configuration [4]. > > > > > > > > The main goal of these PRs is to present and discuss a new approach > for > > > > preparing and managing Ignite configuration in a more robust and > > > convenient > > > > way than it was before. > > > > > > > > These PRs cover basic aspects of configuration but other steps for > > > > developing functionality are already defined; ticket IGNITE-13511 [5] > > > > summarizes work to do. > > > > > > > > In a nutshell proposed approach to configuration is as follows: > > > > > > > > We want to declare configuration with POJO-based schemas that are > > concise > > > > and contain all important information about validation and how > > different > > > > pieces of configuration relate to each other. > > > > When schemas are marked with annotations annotation processor enters > > the > > > > game and generates most of boilerplate code thus freeing users from > > > writing > > > > it by hand. > > > > > > > > REST API module from [2] contains an example of managing > configuration > > > and > > > > exposing it to external tools like a Unified CLI tool presented in > [6]. > > > > > > > > [1] https://github.com/apache/ignite-3/pull/5 > > > > [2] https://github.com/apache/ignite-3/pull/6 > > > > [3] > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-55+Unified+Configuration > > > > [4] > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-63%3A+REST+API+module+to+integrate+new+modular+architecture+and+management > > > > [5] https://issues.apache.org/jira/browse/IGNITE-13511 > > > > [6] > > > > > > > > > > > > > > http://apache-ignite-developers.2346864.n4.nabble.com/DISCUSSION-Unified-CLI-tool-td50618.html > > > > > > > > > >
