aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 41804: Proposal - shim interfaces to preface args system overhaul.
Date Mon, 04 Jan 2016 20:46:04 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message