aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 41804: Proposal - shim interfaces to preface args system overhaul.
Date Thu, 31 Dec 2015 00:18:12 GMT


> On Dec. 30, 2015, 4:14 p.m., John Sirois wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java,
line 123
> > <https://reviews.apache.org/r/41804/diff/1/?file=1179942#file1179942line123>
> >
> >     It might be slightly less churn to move these in to the interface via default
methods - when the cutover is made to the new system where default methods should only provide
defaults, the edits will be line deletes or replacement with default values.

Definitely, i plan to leverage default methods with the new system.


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41804/#review112363
-----------------------------------------------------------


On Dec. 30, 2015, 4:03 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41804/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2015, 4:03 p.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