> On Jan. 23, 2015, 10:18 p.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/config/schema_base.py, lines 69-77
> > <https://reviews.apache.org/r/28920/diff/17-20/?file=823007#file823007line69>
> >
> >     sort of higher-level question -- since this information is not used by 
> > thermos at all, does it make sense in  
> > src/main/python/apache/aurora/config/schema/base.py instead?  usually 
> > Aurora concerns are in Job() and Thermos concerns are in Task().

Are there plan to ever allow more than one Task per Job?  If not, it makes 
sense to move it to the job.  I guess you could always support both in the 
future as well.  I'll move the container onto the job object, it actually makes 
some things much simpler to understand.  For example, currently if you did a 
Tasks.combine, unless both tasks have a container on them, the result is 
confusing.


> On Jan. 23, 2015, 10:18 p.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/executor/common/sandbox.py, line 142
> > <https://reviews.apache.org/r/28920/diff/17-20/?file=823005#file823005line142>
> >
> >     os.chdir is a surprising side-effect for sandbox.create.
> >     
> >     could you instead put this in ThermosTaskRunner.start?  i.e. where we 
> > do self._popen = subprocess.Popen(cmdline_args), Popen takes a cwd= keyword 
> > argument, and it seems like the presence of $MESOS_DIRECTORY should be 
> > enough signal that we should be setting it, and we don't need to chdir here.

I'll give this a try, as long as nowhere else in the executor is using a 
relative path (and I think I got all of them) I think we'll be ok.


- Steve


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


On Jan. 23, 2015, 5:23 p.m., Steve Niemitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28920/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2015, 5:23 p.m.)
> 
> 
> Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-633
>     https://issues.apache.org/jira/browse/AURORA-633
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This change adds support for launching docker containers through aurora.  
> These changes are based off of the discussion in 
> https://issues.apache.org/jira/browse/AURORA-633
> 
> As of now, a special thermos_executor.sh script is needed to launch the 
> executor inside docker containers.  A sample aurora file is in 
> examples/jobs/docker.
> 
> In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
> the example upstart config in examples/vagrant/upstart has been updated to 
> reflect this.
> 
> More information is in subsequent review request comments.
> 
> 
> Diffs
> -----
> 
>   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 08ba1cdf88b712de22c26c04443079282db59ef9 
>   config/legacy_untested_classes.txt 33c1d6eb4ea02e01b7002c2c2bae5a3858c8b0e5 
>   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
>   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
>   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
>   examples/vagrant/aurorabuild.sh f692f025a7e5f2f0dddb7f6c81ea12fcb8272020 
>   examples/vagrant/provision-dev-cluster.sh 
> 40c165925c2110fb727c66ff5a34cf5ab8415343 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 788ec254270bca074dae91829c7f4fccdc8f8bb0 
>   examples/vagrant/upstart/mesos-slave.conf 
> 512ce7ecf34042ed68dda55efb2dd0415f8469db 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8428941c10641857a952f34df4e46a8fac5476a8 
>   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
> 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> d885b224ec5a1d529347d84e03ba98ab6734a126 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  01b03508afac37b5a8f0ec5c3da1460695e1ef59 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 5cc85f1f87f3b8355c89e8ecac19de1122a079e6 
>   src/main/python/apache/aurora/config/thrift.py 
> ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 636b23d30a897b557eb8c3f8733c90b23cb807ef 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> f7d8977e42aa56188799400bf8e12a6886fb4a8f 
>   src/main/python/apache/aurora/executor/common/sandbox.py 
> f47a32b3fefb4a89940b1ddc473b8316ac00df12 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> f29faf1d51baa4af66ad8c6579ffa354409e9536 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/runner.py 
> 41200bd5d74ee3239279567d79a5f48fb6af2e44 
>   src/main/resources/scheduler/assets/configSummary.html 
> 28878908b0c9381e366b71a3135dfc28c542a890 
>   src/main/resources/scheduler/assets/js/services.js 
> b744f375411e09b7f776e4a05ee5961227143439 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fd188fb8f004d0a7664420bfda56568cebedb6f 
>   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
> 876e173ccbac04e4a06a245648c7c6af15eaaa92 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c7de6e111300b009e1f9f430624a56100328184e 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 7eafe074b686d55ad96018006cf4acfa823513c3 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  ad9126c32893080e128d086ea3bfd7ad23d27b89 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> e531fa06e508d9792af51c62e67120c21baa7a81 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 6c8ae1cf7fb578237708a954bd42162d66b39f4d 
> 
> Diff: https://reviews.apache.org/r/28920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Reply via email to