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 > > >
