Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Aurora ReviewBot 

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



Master (73dd2a8) is red with this patch.
  ./build-support/jenkins/build.sh

   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/tmpy92fEM', 'binary', 
'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.80/bin/pants", 
line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  16 failed, 638 passed, 6 skipped, 1 warnings, 8 
error in 108.83 seconds 
 
FAILURE


   Waiting for background workers to finish.
01:29:15 02:31   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


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 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   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/stor

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 

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

(Updated June 27, 2016, 6:18 p.m.)


Review request for Aurora.


Changes
---

Changed URI in api.thrift to MesosFetcherURI


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 (updated)
-

  RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6c7c75ac86458884bc767736caf47fb56fc8 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
4089b79da8079243703eead884e80bcf736f8b29 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 fe18c0f9876a736ea34d4eab92219013cd1e 
  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 (updated)
---

./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

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Aurora ReviewBot 

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


Ship it!




Master (73dd2a8) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 27, 2016, 11:56 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 27, 2016, 11:56 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 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   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
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 


> On June 24, 2016, 4:33 p.m., David McLaughlin wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 266-267
> > 
> >
> > Please rename to something like mesosFetcherUris (or just fetcherUris) 
> > to document purpose of these uris in this context.
> 
> Renan DelValle wrote:
> Sounds like a good idea, I'll get this done after all the technical stuff 
> has been cleared up.

Renamed to mesosFetcherUris.


- Renan


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


On June 27, 2016, 4:56 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 27, 2016, 4:56 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 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   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
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 145
> > 
> >
> > typo

Fixed thanks!


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 146
> > 
> >
> > The name appears to be too generic for its highly specialized use-case. 
> > Perhaps something along the lines of the "MesosFetcherUri" should be a 
> > better fit here?

Since it mirrors the URI in the proto maybe MesosURI would do? I'll change it 
in the next submission if this is fine.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 260
> > 
> >
> > Please revert, there should be a newline after the args spillover.

Done.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 266
> > 
> >
> > Favor java8 stream manipulations over Guava's where possible.
> > 
> > Also, formatting is off, should be:
> > ```
> > Iterable uris = Iterables.transform(
> > task.getTask().getUris(),
> > u -> Protos.CommandInfo.URI.newBuilder()
> > .setValue(u.getValue())
> > .setExecutable(false)
> > .setExtract(u.isExtract())
> > .setCache(u.isCache()).build());
> > ```

Changed to Java 8 streams.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 268
> > 
> >
> > This seems like a candidate for a command line flag documenting the 
> > direct security implications of changing the value here.
> > 
> > Also, along the lines of Joshua's comments, suggest having another 
> > command line flag turning the feature off entirely (default) with a check 
> > in ConfigurationManager rejecting any tasks with fetcher URIs set.

One flag to allow the executable field to be toggled and another to allow URIs 
to be fetched right? I've been playing it safe and not allowing
the user a choice for now to set anything as executable as anything that needs 
to be downloaded to the sandbox and marked executable can be set to be fetched 
in the static server side config (custom executor config). I can add the 
ability to toggle being able to mark URIs downlaoded exectuable otherwise 
though.

I've added the flag to turn this feature off entirely and have set it to be the 
default.


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java, 
> > lines 155-157
> > 
> >
> > Fits on a single line.

Changed, thanks!


> On June 27, 2016, 9:44 a.m., Maxim Khutornenko wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml,
> >  line 406
> > 
> >
> > Please, add/modify AbstractTaskStoreTest to test this codepath.

Added, will iterate if it's not good enough.


- Renan


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


On June 27, 2016, 4:56 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 27, 2016, 4:56 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 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6c7c75ac86458884bc767736caf47fb56fc8 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  fe18c0f9876a736ea34d4eab92219013cd1e 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskF

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Renan DelValle 

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

(Updated June 27, 2016, 4:56 p.m.)


Review request for Aurora.


Changes
---

Added flag -enable_mesos_fetcher_for_jobs to control wether arbitrary URIs 
provided by the client are allowed or not.
Renamed uris to mesosFetcherUris. 
Added tests for mesosFetcherUris in AbstractTaskStoreTest. 
Changed from Guava to to Java 8 streams.


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 (updated)
-

  RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
6c7c75ac86458884bc767736caf47fb56fc8 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
4089b79da8079243703eead884e80bcf736f8b29 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 fe18c0f9876a736ea34d4eab92219013cd1e 
  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
./build-support/jenkins/build.sh
bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Renan DelValle

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Mehrdad Nurolahzade 


> On June 27, 2016, 11:31 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/context.py, lines 129-130
> > 
> >
> > Can you simplify this as:
> > ```
> > return not to_bool(tier.settings['preemptible']) and not 
> > to_bool(tier.settings['revocable'])
> > ```
> > 
> > and falback to `tier_configurations.defaultTierName` in the next()?
> > 
> > Also, I'd change the function name to something less generic, like 
> > `production_tier_filter`.
> 
> Mehrdad Nurolahzade wrote:
> Do you mean something like the following? (this is breaking my tests)
> ```
> def production_tier_filter(tier):
>   return not to_bool(tier.settings['preemptible']) and not 
> to_bool(tier.settings['revocable'])
> 
> task = config.job().taskConfig
> if task.tier is None:
>   backfill_args = {
> 'tier': String(
>   next(
> (t.name for t in tier_configurations.tiers if 
> production_tier_filter(t)),
> tier_configurations.defaultTierName))
>   }
> else:
>   backfill_args = {
> 'production': Boolean(
>   next(
> (not to_bool(t.settings['preemptible']) for t in 
> tier_configurations.tiers if
>   production_tier_filter(t)),
> task.production))
>   }
> ```
> 
> Maxim Khutornenko wrote:
> I presume this is due to the "else" block not accounting for the actual 
> `task.tier` value?

Yes, and the if block also relies on ```task.production``` value. 
I am not sure if I am following your suggested simplification of 
```production_tier_filter``` and using it in both if and else blocks:
```
def production_tier_filter(tier):
  return not to_bool(tier.settings['preemptible']) and not 
to_bool(tier.settings['revocable'])
```


- Mehrdad


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


On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 9:02 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9b1511801d031ff48b81c25688a55cb586b8ac66 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 2130f1fa71be02a004cdf8e476a270c81a7105d3 
>   src/test/python/apache/aurora/client/cli/test_context.py 
> 204ca092adad8bf43c5032a02f61bf303fb0b2fc 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f3c522ed94a2d774865811ceb546bf9df083c14f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> a545fece5e2b3e0017a61e1be9ac478372b1f34d 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 967d560e5c7eb0ed85b215fb11d9751b8666acb5 
>   src/test/python/apache/aurora/client/cli/util.py 
> 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
> 
> Diff: https://reviews.apache.org/r/49048/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> *** OK (All tests passed) ***
> 
> mesos-master start/running, process 26868
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> real  19m46.324s
> user  0m1.496s
> sys   0m0.774s
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Maxim Khutornenko 


> On June 27, 2016, 6:31 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/context.py, lines 129-130
> > 
> >
> > Can you simplify this as:
> > ```
> > return not to_bool(tier.settings['preemptible']) and not 
> > to_bool(tier.settings['revocable'])
> > ```
> > 
> > and falback to `tier_configurations.defaultTierName` in the next()?
> > 
> > Also, I'd change the function name to something less generic, like 
> > `production_tier_filter`.
> 
> Mehrdad Nurolahzade wrote:
> Do you mean something like the following? (this is breaking my tests)
> ```
> def production_tier_filter(tier):
>   return not to_bool(tier.settings['preemptible']) and not 
> to_bool(tier.settings['revocable'])
> 
> task = config.job().taskConfig
> if task.tier is None:
>   backfill_args = {
> 'tier': String(
>   next(
> (t.name for t in tier_configurations.tiers if 
> production_tier_filter(t)),
> tier_configurations.defaultTierName))
>   }
> else:
>   backfill_args = {
> 'production': Boolean(
>   next(
> (not to_bool(t.settings['preemptible']) for t in 
> tier_configurations.tiers if
>   production_tier_filter(t)),
> task.production))
>   }
> ```

I presume this is due to the "else" block not accounting for the actual 
`task.tier` value?


> On June 27, 2016, 6:31 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/config.py, line 118
> > 
> >
> > Please, add a link to our docs for more info: 
> > http://aurora.apache.org/documentation/latest/reference/configuration/#job-objects
> > 
> > Also, release notes need to be updated to clearly state this 
> > deprecation route.
> 
> Mehrdad Nurolahzade wrote:
> We added an entry to ```RELEASE-NOTES.md``` for 0.14.0 under 
> "Deprecations and removals" with a previous JIRA, do we need anything beyond 
> that?
> ```
> - Deprecated `production` field in `TaskConfig` thrift struct. Use `tier` 
> field to specify task
>   scheduling and resource handling behavior.
> ```

I would add a separate note explicitly stating the new client behavior: warning 
message and tier defaulting. This is finally a user-visible change that I think 
warrants a special mentioning.


- Maxim


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


On June 27, 2016, 4:02 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 4:02 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9b1511801d031ff48b81c25688a55cb586b8ac66 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 2130f1fa71be02a004cdf8e476a270c81a7105d3 
>   src/test/python/apache/aurora/client/cli/test_context.py 
> 204ca092adad8bf43c5032a02f61bf303fb0b2fc 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f3c522ed94a2d774865811ceb546bf9df083c14f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> a545fece5e2b3e0017a61e1be9ac478372b1f34d 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 967d560e5c7eb0ed85b215fb11d9751b8666acb5 
>   src/test/python/apache/aurora/client/cli/util.py 
> 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
> 
> Diff: https://reviews.apache.org/r/49048/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> *** OK (All tests passed) ***
> 
> mesos-master start/running, process 26868
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> real  19m46.324s
> user  0m1.496s
> sys   0m0.774s
> ```
> 
> 
> Thanks,
> 
> Mehr

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Mehrdad Nurolahzade 


> On June 27, 2016, 11:31 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/config.py, line 118
> > 
> >
> > Please, add a link to our docs for more info: 
> > http://aurora.apache.org/documentation/latest/reference/configuration/#job-objects
> > 
> > Also, release notes need to be updated to clearly state this 
> > deprecation route.

We added an entry to ```RELEASE-NOTES.md``` for 0.14.0 under "Deprecations and 
removals" with a previous JIRA, do we need anything beyond that?
```
- Deprecated `production` field in `TaskConfig` thrift struct. Use `tier` field 
to specify task
  scheduling and resource handling behavior.
```


- Mehrdad


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


On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 9:02 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9b1511801d031ff48b81c25688a55cb586b8ac66 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 2130f1fa71be02a004cdf8e476a270c81a7105d3 
>   src/test/python/apache/aurora/client/cli/test_context.py 
> 204ca092adad8bf43c5032a02f61bf303fb0b2fc 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f3c522ed94a2d774865811ceb546bf9df083c14f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> a545fece5e2b3e0017a61e1be9ac478372b1f34d 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 967d560e5c7eb0ed85b215fb11d9751b8666acb5 
>   src/test/python/apache/aurora/client/cli/util.py 
> 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
> 
> Diff: https://reviews.apache.org/r/49048/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> *** OK (All tests passed) ***
> 
> mesos-master start/running, process 26868
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> real  19m46.324s
> user  0m1.496s
> sys   0m0.774s
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Mehrdad Nurolahzade 


> On June 27, 2016, 11:31 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/cli/context.py, lines 129-130
> > 
> >
> > Can you simplify this as:
> > ```
> > return not to_bool(tier.settings['preemptible']) and not 
> > to_bool(tier.settings['revocable'])
> > ```
> > 
> > and falback to `tier_configurations.defaultTierName` in the next()?
> > 
> > Also, I'd change the function name to something less generic, like 
> > `production_tier_filter`.

Do you mean something like the following? (this is breaking my tests)
```
def production_tier_filter(tier):
  return not to_bool(tier.settings['preemptible']) and not 
to_bool(tier.settings['revocable'])

task = config.job().taskConfig
if task.tier is None:
  backfill_args = {
'tier': String(
  next(
(t.name for t in tier_configurations.tiers if 
production_tier_filter(t)),
tier_configurations.defaultTierName))
  }
else:
  backfill_args = {
'production': Boolean(
  next(
(not to_bool(t.settings['preemptible']) for t in 
tier_configurations.tiers if
  production_tier_filter(t)),
task.production))
  }
```


- Mehrdad


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


On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 9:02 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9b1511801d031ff48b81c25688a55cb586b8ac66 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 2130f1fa71be02a004cdf8e476a270c81a7105d3 
>   src/test/python/apache/aurora/client/cli/test_context.py 
> 204ca092adad8bf43c5032a02f61bf303fb0b2fc 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f3c522ed94a2d774865811ceb546bf9df083c14f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> a545fece5e2b3e0017a61e1be9ac478372b1f34d 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 967d560e5c7eb0ed85b215fb11d9751b8666acb5 
>   src/test/python/apache/aurora/client/cli/util.py 
> 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
> 
> Diff: https://reviews.apache.org/r/49048/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> *** OK (All tests passed) ***
> 
> mesos-master start/running, process 26868
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> real  19m46.324s
> user  0m1.496s
> sys   0m0.774s
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Maxim Khutornenko 

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




src/main/python/apache/aurora/client/api/__init__.py (line 298)


This should be `log.debug()` instead as this message does not add much 
value in general case.



src/main/python/apache/aurora/client/cli/context.py (lines 129 - 130)


Can you simplify this as:
```
return not to_bool(tier.settings['preemptible']) and not 
to_bool(tier.settings['revocable'])
```

and falback to `tier_configurations.defaultTierName` in the next()?

Also, I'd change the function name to something less generic, like 
`production_tier_filter`.



src/main/python/apache/aurora/client/cli/context.py (line 141)


`job_update` had me pause for a moment before I realized it's not about job 
updates. How about something like `backfill_args` instead?



src/main/python/apache/aurora/client/cli/context.py (line 144)


I think you can reuse the `settings_filter` above if you modify it as 
suggested above and fallback to `task.production`.



src/main/python/apache/aurora/client/config.py (line 118)


Please, add a link to our docs for more info: 
http://aurora.apache.org/documentation/latest/reference/configuration/#job-objects

Also, release notes need to be updated to clearly state this deprecation 
route.



src/main/python/apache/aurora/client/config.py (line 124)


Can you use log.warning() instead? This would simplify unit tests and avoid 
relying on StringIO to validate output. 

There used to be a `deprecation_warning()` method in base.py dropped in 
this RB: https://reviews.apache.org/r/39958. You can reinstate it in this 
module.


- Maxim Khutornenko


On June 27, 2016, 4:02 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 4:02 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9b1511801d031ff48b81c25688a55cb586b8ac66 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 2130f1fa71be02a004cdf8e476a270c81a7105d3 
>   src/test/python/apache/aurora/client/cli/test_context.py 
> 204ca092adad8bf43c5032a02f61bf303fb0b2fc 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f3c522ed94a2d774865811ceb546bf9df083c14f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> a545fece5e2b3e0017a61e1be9ac478372b1f34d 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 967d560e5c7eb0ed85b215fb11d9751b8666acb5 
>   src/test/python/apache/aurora/client/cli/util.py 
> 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
> 
> Diff: https://reviews.apache.org/r/49048/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> *** OK (All tests passed) ***
> 
> mesos-master start/running, process 26868
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> real  19m46.324s
> user  0m1.496s
> sys   0m0.774s
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Aurora ReviewBot 

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


Ship it!




Master (73dd2a8) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 27, 2016, 4:02 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 4:02 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9b1511801d031ff48b81c25688a55cb586b8ac66 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 2130f1fa71be02a004cdf8e476a270c81a7105d3 
>   src/test/python/apache/aurora/client/cli/test_context.py 
> 204ca092adad8bf43c5032a02f61bf303fb0b2fc 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f3c522ed94a2d774865811ceb546bf9df083c14f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> a545fece5e2b3e0017a61e1be9ac478372b1f34d 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 967d560e5c7eb0ed85b215fb11d9751b8666acb5 
>   src/test/python/apache/aurora/client/cli/util.py 
> 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
> 
> Diff: https://reviews.apache.org/r/49048/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> *** OK (All tests passed) ***
> 
> mesos-master start/running, process 26868
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> real  19m46.324s
> user  0m1.496s
> sys   0m0.774s
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Mehrdad Nurolahzade 

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



@ReviewBot retry

- Mehrdad Nurolahzade


On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 9:02 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/context.py 
> 9b1511801d031ff48b81c25688a55cb586b8ac66 
>   src/main/python/apache/aurora/client/config.py 
> 2fc12559016d406c347adb416a5166cca31c961e 
>   src/test/python/apache/aurora/client/cli/test_command_hooks.py 
> 2130f1fa71be02a004cdf8e476a270c81a7105d3 
>   src/test/python/apache/aurora/client/cli/test_context.py 
> 204ca092adad8bf43c5032a02f61bf303fb0b2fc 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f3c522ed94a2d774865811ceb546bf9df083c14f 
>   src/test/python/apache/aurora/client/cli/test_plugins.py 
> a545fece5e2b3e0017a61e1be9ac478372b1f34d 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> 967d560e5c7eb0ed85b215fb11d9751b8666acb5 
>   src/test/python/apache/aurora/client/cli/util.py 
> 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
>   src/test/python/apache/aurora/client/test_config.py 
> b1a3c1865819899ef19173be0f861783a2631d0a 
> 
> Diff: https://reviews.apache.org/r/49048/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> *** OK (All tests passed) ***
> 
> mesos-master start/running, process 26868
> + RETCODE=0
> + restore_netrc
> + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
> + true
> Connection to 127.0.0.1 closed.
> 
> real  19m46.324s
> user  0m1.496s
> sys   0m0.774s
> ```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>

Re: Review Request 49218: Add support for Mesos Fetcher

2016-06-27 Thread Maxim Khutornenko 

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




api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 145)


typo



api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 146)


The name appears to be too generic for its highly specialized use-case. 
Perhaps something along the lines of the "MesosFetcherUri" should be a better 
fit here?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 


Please revert, there should be a newline after the args spillover.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 265)


Favor java8 stream manipulations over Guava's where possible.

Also, formatting is off, should be:
```
Iterable uris = Iterables.transform(
task.getTask().getUris(),
u -> Protos.CommandInfo.URI.newBuilder()
.setValue(u.getValue())
.setExecutable(false)
.setExtract(u.isExtract())
.setCache(u.isCache()).build());
```



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 267)


This seems like a candidate for a command line flag documenting the direct 
security implications of changing the value here.

Also, along the lines of Joshua's comments, suggest having another command 
line flag turning the feature off entirely (default) with a check in 
ConfigurationManager rejecting any tasks with fetcher URIs set.



src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
(lines 155 - 157)


Fits on a single line.



src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
(line 406)


Please, add/modify AbstractTaskStoreTest to test this codepath.


- Maxim Khutornenko


On June 24, 2016, 11:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49218/
> ---
> 
> (Updated June 24, 2016, 11:01 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
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3e6daf53dd563dd7a2d494cc95e9a0aba0b6 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 4089b79da8079243703eead884e80bcf736f8b29 
>   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_CreateURIsTable.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/mesos/MesosTaskFactoryImplTest.java 
> 58785bfa37ff214f26e9f94d836e6df40e411c3b 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> a883b0e33bfec1d14e6fe4ee8ed2200d93acaeec 
> 
> Diff: https://reviews.apache.org/r/49218/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> ./build-support/jenkins/build.sh
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Mehrdad Nurolahzade 


> On June 27, 2016, 9:22 a.m., Aurora ReviewBot wrote:
> > Master (73dd2a8) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  ERROR] Could not quitquitquit runner: Cannot take 
> > control of a task in terminal state.
> >  E0627 16:21:42.444761 9324 thermos_task_runner.py:234] 
> > Could not quitquitquit runner: Cannot take control of a task in terminal 
> > state.
> >   generated xml file: 
> > /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
> >  
> >   1 failed, 667 passed, 6 skipped, 1 warnings 
> > in 242.05 seconds 
> >  
> > FAILURE
> > 
> > 
> >Waiting for background workers to finish.
> > 16:22:36 04:46   [complete]
> >FAILURE
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

@ReviewBot retry


- Mehrdad


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


On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 9:02 a.m.)
> 
> 
> Review request

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Aurora ReviewBot 

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



Master (73dd2a8) is red with this patch.
  ./build-support/jenkins/build.sh

 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 E0627 16:21:42.444761 9324 thermos_task_runner.py:234] 
Could not quitquitquit runner: Cannot take control of a task in terminal state.
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 667 passed, 6 skipped, 1 warnings in 
242.05 seconds 
 
FAILURE


   Waiting for background workers to finish.
16:22:36 04:46   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On June 27, 2016, 4:02 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49048/
> ---
> 
> (Updated June 27, 2016, 4:02 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
> Job configuration - CLI changes
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 68baf8fdb90cd26100159401c46c9963c24332b3 
>   src/main/python/apache/aurora/client/cli/

Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes

2016-06-27 Thread Mehrdad Nurolahzade 

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

(Updated June 27, 2016, 9:02 a.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

Applied suggested refactorings


Repository: aurora


Description
---

AURORA-1710 Make 'tier' required and remove support for 'production' flag in 
Job configuration - CLI changes


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/__init__.py 
68baf8fdb90cd26100159401c46c9963c24332b3 
  src/main/python/apache/aurora/client/cli/context.py 
9b1511801d031ff48b81c25688a55cb586b8ac66 
  src/main/python/apache/aurora/client/config.py 
2fc12559016d406c347adb416a5166cca31c961e 
  src/test/python/apache/aurora/client/cli/test_command_hooks.py 
2130f1fa71be02a004cdf8e476a270c81a7105d3 
  src/test/python/apache/aurora/client/cli/test_context.py 
204ca092adad8bf43c5032a02f61bf303fb0b2fc 
  src/test/python/apache/aurora/client/cli/test_create.py 
8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c 
  src/test/python/apache/aurora/client/cli/test_cron.py 
f3c522ed94a2d774865811ceb546bf9df083c14f 
  src/test/python/apache/aurora/client/cli/test_plugins.py 
a545fece5e2b3e0017a61e1be9ac478372b1f34d 
  src/test/python/apache/aurora/client/cli/test_restart.py 
967d560e5c7eb0ed85b215fb11d9751b8666acb5 
  src/test/python/apache/aurora/client/cli/util.py 
7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e 
  src/test/python/apache/aurora/client/test_config.py 
b1a3c1865819899ef19173be0f861783a2631d0a 

Diff: https://reviews.apache.org/r/49048/diff/


Testing
---

```
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

*** OK (All tests passed) ***

mesos-master start/running, process 26868
+ RETCODE=0
+ restore_netrc
+ mv /home/vagrant/.netrc.bak /home/vagrant/.netrc
+ true
Connection to 127.0.0.1 closed.

real19m46.324s
user0m1.496s
sys 0m0.774s
```


Thanks,

Mehrdad Nurolahzade