> On March 11, 2016, 8:13 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java, lines 91-93
> > <https://reviews.apache.org/r/44685/diff/2/?file=1295228#file1295228line91>
> >
> >     I feel like this is confusingly named. The scheduler already accepts 
> > tasks that use the Docker containerizer. What this patch actually does is 
> > allow for tasks to run in the Docker containerizer without the need for an 
> > executor to coordinate their execution, right?
> >     
> >     Is the term "Docker Containerizer" somehow overloaded and leading to my 
> > confusion?
> >     
> >     How would you feel about renaming this to something like 
> > `require_docker_use_executor` (defaulting to True)?
> >     
> >     Open to other ideas as well, but figured I'd throw one out to start.

You're right.  The naming doesn't help, and there are multiple overlapping 
paths to the same docker mechanisms in mesos.  I like your arg name better, 
changed.


> On March 11, 2016, 8:13 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java,
> >  lines 281-282
> > <https://reviews.apache.org/r/44685/diff/2/?file=1295230#file1295230line281>
> >
> >     This read a little bit better to me the previous way it was written. 
> > Maybe something like: `This scheduler is not configured to allow the 
> > container type: " + containerType.get().toString()`?

Done.  I did the rewording to make it clear that there is no fundamental 
problem, but the denial is due to configuration.


> On March 11, 2016, 8:13 a.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 164
> > <https://reviews.apache.org/r/44685/diff/2/?file=1295231#file1295231line164>
> >
> >     Instead of building the `TaskInfo` above only to clear it all out in 
> > this case, how about just extracting the logic to build the TaskInfo for 
> > tasks w/ an executor, and invoking that in the branches where it's used?

Thanks for the nudge.  I didn't do this for fear of repetitive code, but i 
found a single branch to do it in.


- Bill


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


On March 10, 2016, 6:24 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44685/
> -----------------------------------------------------------
> 
> (Updated March 10, 2016, 6:24 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is currently labeled as experimental.
> 
> Only the most basic wiring is added here, and assumes that the provided image
> includes an ENTRYPOINT.  Unlike Docker support via the thermos executor, this
> approach allows containers with an entrypoint, and does not impose environment
> requirements on the image (e.g. python interpreter, libmesos dependencies).
> 
> Note that when using this, other familiar Aurora facilities that relate to the
> thermos executor will not work.  For example, browsing task logs is not
> supported.
> 
> Support for exercising this from the client will come shortly.
> 
> 
> Diffs
> -----
> 
>   NEWS da3e4cea8ca688b6b7c5bafae67133df065d9255 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> 60746383fccb107ca27925a91aa1803e2cf0fd85 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> a0d2a717534bbb2e85a556721cc53c1e4b743461 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 1de6966565d2fbd9abd220ad8162b624b109959a 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  e700fa3550312bfa9c8a3adb25d135f6f500c4b5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> a34af4d2fb3863ab8197bcdce942c513d629621b 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  11062e3a097e490c61bfd4dc84990903275521a3 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 3db531b52fb2bd94b4b5ce62e6554b5a85ed3ea8 
> 
> Diff: https://reviews.apache.org/r/44685/diff/
> 
> 
> Testing
> -------
> 
> Via additional hacking, i successfully ran the stock [hello 
> world](https://hub.docker.com/_/hello-world/) image.  Within the sandbox, i 
> observed the expected output in the `stdout` file.  Status updates for the 
> task exiting worked as expected.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to