----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41804/#review112643 -----------------------------------------------------------
Has the high level plan been discussed on dev list before or is it the first time this proposal is shaped up? It would be great to have an official design summary or at least a ticket capturing the goals of this refactoring. - Maxim Khutornenko On Dec. 31, 2015, 12:03 a.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41804/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2015, 12:03 a.m.) > > > Review request for Aurora, John Sirois and Zameer Manji. > > > Repository: aurora > > > Description > ------- > > This begins to define a proposed replacement args API, from the perspective > of the code consuming args. Args will be defined in interfaces, which the > eventual arg system will be responsible for implementing on the fly > (mechanism TBD). So while what is seen here is a large net increase in code, > the eventual conclusion will be roughly equivalent in terms of lines of code > in `Module`s. > > There are a few goals with the replacement: > - sidestep current development hurdles we have encountered with the args > system (intellij/gradle not working nicely with apt) > - leverage a well-maintained third-party argument parsing library > - encourage better testability of Module classes by always injecting all args > - enable user-friendly features like logical option groups for better > help/usage output > - stretch: enable alternative configuration inputs like a configuration file > or environment variables > > The rough plan of action is as follows (if the proposal looks good): > 1. repeat this patch for all other `@CmdLine` declaration sites (28 files) > 2. introduce a 'boot' `Injector` that is loaded with bindings for these > params implementations (i.e. centralize the `new ExecutorModuleParams() { .. > }` boilerplate you see here > 3. replace the args backend > (a) remove `@CmdLine Arg<>` declarations, moving help text to annotations > on interface methods > (b) implement a system to inject proxies that implement params classes > based on arg values, and binds them for injection > > Given that this is a large multi-stage effort, i may opt to implement it on a > branch and land it all at once in a stream of commits to avoid > churn/confusion in the meantime. Open to thoughts on that. > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java > 949c299bdbc54f976db994266fb97f3099256f13 > > Diff: https://reviews.apache.org/r/41804/diff/ > > > Testing > ------- > > > Thanks, > > Bill Farner > >
