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



src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
<https://reviews.apache.org/r/31338/#comment120136>

    `Arg.<List<String>>create(...)` should let you avoid the cast?
    
    That said, since you're actually splitting the arg values, I think you 
should be able to use a Map instead to simplify the parsing logic below a bit.
    
    c.f. 
https://github.com/apache/incubator-aurora/blob/e6e7e53d92b52d78960824022bef8a0546002180/src/main/java/org/apache/aurora/scheduler/thrift/auth/ThriftAuthModule.java#L43
    
    The accompanying command line format would be comma delimited key=value 
pairs. You could still use a delimiter to allow for specifying the (optional) 
mode.



src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
<https://reviews.apache.org/r/31338/#comment120139>

    If we validate in the guice module as suggested below, then perhaps we can 
inject a parsed map of host => (path, mode) values (maybe create a small bean 
to model the mount point). That way we don't have to re-parse the values in the 
task factory below.



src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
<https://reviews.apache.org/r/31338/#comment120138>

    I think it would make more sense to validate these mounts in the Guice 
module where the arg is defined? That way instead of throwing an 
IllegalArgumentException at runtime, we could use Guice's addError method to 
have the problem reported at start up as a normal binding problem.



src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
<https://reviews.apache.org/r/31338/#comment120137>

    Extract constants for RW and RO? Maybe a(n Immutable) set? Or perhaps reuse 
the constants on Volume used below? Then just ensure the value is a member of 
the set and we can also just join the values of the set to generate the error 
message if necessary.


- Joshua Cohen


On Feb. 24, 2015, 2:07 a.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31338/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2015, 2:07 a.m.)
> 
> 
> Review request for Aurora, Jay Buffington and Bill Farner.
> 
> 
> Bugs: AURORA-1107
>     https://issues.apache.org/jira/browse/AURORA-1107
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added a command line flag, -global_container_mounts, to allow mounting paths 
> from the slaves into the (docker) containers they run.
> 
> This is the first portion of allowing per-job mounts, however, I wanted to 
> get this out first since more people want it.  I'll implement per-job mounts 
> in a future review.
> 
> 
> Diffs
> -----
> 
>   docs/deploying-aurora-scheduler.md d1123359961fd59ddb8c1a07f80f293bdd46019f 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> bacfbfeb237ecddf82f58679e05be012c5214e61 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 24b61c1e4f615295acf28d904588e1512972d3f4 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> baacb71403d55c5b90fc11cb2a23f552a32e8ba5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5340d651b298ec8aa079e73d6d2f652fdf876293 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 6575b7d420f17ec68d6e2a83e7b380f684577d4f 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 444d6d3fdaf86eb84612f846eaa326eb75c49898 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> efe62ceb502ead88a2f0cd6d09a76664e465d9bc 
> 
> Diff: https://reviews.apache.org/r/31338/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Reply via email to