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



LGTM, just a few minor changes left.


api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 148 - 152)
<https://reviews.apache.org/r/49218/#comment205163>

    Holes in thrift IDs are legal but since this is a new struct it'd make 
sense to avoid them.



src/main/java/org/apache/aurora/scheduler/app/AppModule.java (line 100)
<https://reviews.apache.org/r/49218/#comment205167>

    I'd skip `_for_jobs` part in favor of a brief write up under 
https://github.com/apache/aurora/tree/master/docs/features describing custom 
executors and mesos fetcher.



src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (line 225)
<https://reviews.apache.org/r/49218/#comment205168>

    indent is off here



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 
268 - 272)
<https://reviews.apache.org/r/49218/#comment205169>

    Suggest a small reformatting here:
    ```
    List<CommandInfo.URI> mesosFetcherUris = 
task.getTask().getMesosFetcherUris().stream()
        .map(u -> Protos.CommandInfo.URI.newBuilder().setValue(u.getValue())
            .setExecutable(false)
            .setExecutable(false)
            .setExtract(u.isExtract())
            .setCache(u.isCache()).build())
        .collect(Collectors.toList());
    ```


- Maxim Khutornenko


On June 28, 2016, 1:18 a.m., Renan DelValle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> -----------------------------------------------------------
> 
> (Updated June 28, 2016, 1:18 a.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 
>   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