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


Fix it, then Ship it!




lgtm modulo the below. Please fix or drop issues as warranted and then I can 
commit this.


src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 267)
<https://reviews.apache.org/r/49218/#comment205262>

    Is there a mesos ticket tracking this? If so, can you add it here?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 
270 - 271)
<https://reviews.apache.org/r/49218/#comment205263>

    Duplicate calls to `setExecutable`?



src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 (line 301)
<https://reviews.apache.org/r/49218/#comment205265>

    It's hard to tell from looking at the review, do we have the inverse of 
this scenario covered as well (i.e. mesos fetcher is disabled and task config 
has no mesos fetcher uris set)? If not, would you mind adding coverage for that?


- Joshua Cohen


On June 28, 2016, 9:39 p.m., Renan DelValle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> -----------------------------------------------------------
> 
> (Updated June 28, 2016, 9:39 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding a URIs field to TaskConfig inside the ThriftAPI so that users are able 
> to specify resources they wish to download into the sandbox per job.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf444453dd563dd7a2d494cc95e9a0aba0b6 
>   docs/features/mesos-fetcher.md PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb777756fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a73eeee6ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 3b01801d929dd61ee989495bf38af8f03e9f5ad4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> c76164292cf62d2181374c09f8bf6d8d3358e982 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 571201094c1e576e496495a01cb83f6c57decfa8 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V007_CreateMesosFetcherURIsTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> a90cb00e240df25dce6d55728859768e22d741a6 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  2c8af8b88e41b3b381cac831fd43b1057e4df0aa 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 5069bedc08bb7111d0e0f101c8a2c81495b97bc9 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  2dff80b5213e98c778d71955517e5f9227d7d0c1 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> b1593f682f48ea66339bc2372de3e4f14e40be32 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> ed69996593f5556d80a9229063ef5c7d658e2cfb 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build -Pq
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> Tested with a custom client submitting TaskConfigs which included URIs when 
> the -enable_mesos_fetcher_for_jobs flag was on as well as when it was off.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Reply via email to