Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-23 Thread Stephan Erb

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


Ship it!




Ship It!

- Stephan Erb


On Oct. 23, 2017, 4:14 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 23, 2017, 4:14 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
>   docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 54d7d4ce0e93ca2278bb5f176e633ad72991ee9d 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 60bbe39d1282ccaa1bb91ad595412d55d115e6c5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b7f5e35079431d26065bc95430badb3e6d229a2a 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  265e87ea9ec0bf3a73a1b5e9d7c40e3d9c79e863 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 459d6bebcd7d6341dac2aead7e3dd8ce87bc9ed6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2cd19d572d2945e28630b9bce5ba58eb9753630a 
>   src/test/python/apache/aurora/client/test_config.py 
> 042372e4d73bee9cedb7f0fc5cfd009bc766aee2 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-23 Thread Aurora ReviewBot

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


Ship it!




Master (dbe9a52) 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 Oct. 23, 2017, 2:14 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 23, 2017, 2:14 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
>   docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 54d7d4ce0e93ca2278bb5f176e633ad72991ee9d 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 60bbe39d1282ccaa1bb91ad595412d55d115e6c5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b7f5e35079431d26065bc95430badb3e6d229a2a 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  265e87ea9ec0bf3a73a1b5e9d7c40e3d9c79e863 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 459d6bebcd7d6341dac2aead7e3dd8ce87bc9ed6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2cd19d572d2945e28630b9bce5ba58eb9753630a 
>   src/test/python/apache/aurora/client/test_config.py 
> 042372e4d73bee9cedb7f0fc5cfd009bc766aee2 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-23 Thread Mauricio Garavaglia

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

(Updated Oct. 23, 2017, 2:14 p.m.)


Review request for Aurora and Stephan Erb.


Repository: aurora


Description
---

Moves the job environment validation to the scheduler, which can be enabled 
with the scheduler require_predefined_environments flag. This allows to have a 
consistent behavior when using the CLI and the API. In order to preserve 
backward compatibility, the validation is kept in the CLI and for the API it 
needs to be manually enabled in the scheduler.


Diffs (updated)
-

  RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
  docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
  docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
  docs/reference/scheduler-configuration.md 
4e3f90713c307e3b9e9f84c29343af7f014f0165 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
54d7d4ce0e93ca2278bb5f176e633ad72991ee9d 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
60bbe39d1282ccaa1bb91ad595412d55d115e6c5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b7f5e35079431d26065bc95430badb3e6d229a2a 
  src/main/python/apache/aurora/client/config.py 
70c2c980309e18de576b251087cdfea00ac06b75 
  src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 265e87ea9ec0bf3a73a1b5e9d7c40e3d9c79e863 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
459d6bebcd7d6341dac2aead7e3dd8ce87bc9ed6 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
3c5ecd698557cafdf8eeacdc472589a379018896 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
2cd19d572d2945e28630b9bce5ba58eb9753630a 
  src/test/python/apache/aurora/client/test_config.py 
042372e4d73bee9cedb7f0fc5cfd009bc766aee2 


Diff: https://reviews.apache.org/r/62692/diff/7/

Changes: https://reviews.apache.org/r/62692/diff/6-7/


Testing
---


Thanks,

Mauricio Garavaglia



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-14 Thread Aurora ReviewBot

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


Ship it!




Master (2aee90d) 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 Oct. 14, 2017, 11:29 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 14, 2017, 11:29 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
>   docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 54d7d4ce0e93ca2278bb5f176e633ad72991ee9d 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 60bbe39d1282ccaa1bb91ad595412d55d115e6c5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b7f5e35079431d26065bc95430badb3e6d229a2a 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  265e87ea9ec0bf3a73a1b5e9d7c40e3d9c79e863 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 459d6bebcd7d6341dac2aead7e3dd8ce87bc9ed6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2cd19d572d2945e28630b9bce5ba58eb9753630a 
>   src/test/python/apache/aurora/client/test_config.py 
> 042372e4d73bee9cedb7f0fc5cfd009bc766aee2 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-14 Thread Mauricio Garavaglia

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

(Updated Oct. 14, 2017, 11:29 p.m.)


Review request for Aurora and Stephan Erb.


Repository: aurora


Description
---

Moves the job environment validation to the scheduler, which can be enabled 
with the scheduler require_predefined_environments flag. This allows to have a 
consistent behavior when using the CLI and the API. In order to preserve 
backward compatibility, the validation is kept in the CLI and for the API it 
needs to be manually enabled in the scheduler.


Diffs (updated)
-

  docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
  docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
  docs/reference/scheduler-configuration.md 
4e3f90713c307e3b9e9f84c29343af7f014f0165 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
54d7d4ce0e93ca2278bb5f176e633ad72991ee9d 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
60bbe39d1282ccaa1bb91ad595412d55d115e6c5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b7f5e35079431d26065bc95430badb3e6d229a2a 
  src/main/python/apache/aurora/client/config.py 
70c2c980309e18de576b251087cdfea00ac06b75 
  src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 265e87ea9ec0bf3a73a1b5e9d7c40e3d9c79e863 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
459d6bebcd7d6341dac2aead7e3dd8ce87bc9ed6 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
3c5ecd698557cafdf8eeacdc472589a379018896 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
2cd19d572d2945e28630b9bce5ba58eb9753630a 
  src/test/python/apache/aurora/client/test_config.py 
042372e4d73bee9cedb7f0fc5cfd009bc766aee2 


Diff: https://reviews.apache.org/r/62692/diff/6/

Changes: https://reviews.apache.org/r/62692/diff/5-6/


Testing
---


Thanks,

Mauricio Garavaglia



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-14 Thread Aurora ReviewBot

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



Master (2aee90d) is red with this patch.
  ./build-support/jenkins/build.sh

 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_process_cannot_depend_upon_daemon
 PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_non_ephemeral_process_cannot_depend_on_ephemeral_process
 PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_failed_predecessor_does_not_make_process_runnable
 PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_daemon_duration
 PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_waits 
PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_fails 
PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_lost 
PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_filters 
PASSED
 
src/test/python/apache/thermos/common/test_task_planner.py::test_task_max_runs 
PASSED
 
src/test/python/apache/thermos/common/test_pathspec.py::test_legacy_task_roots 
PASSED
 
src/test/python/apache/thermos/common/test_pathspec.py::test_legacy_log_dirs 
PASSED
 
src/test/python/apache/thermos/common/test_pathspec.py::test_exception_on_none_keys
 PASSED
 
src/test/python/apache/thermos/common/test_planner.py::test_planner_empty 
PASSED
 
src/test/python/apache/thermos/common/test_planner.py::test_planner_unordered 
PASSED
 
src/test/python/apache/thermos/common/test_planner.py::test_planner_ordered 
PASSED
 
src/test/python/apache/thermos/common/test_planner.py::test_planner_mixed 
PASSED
 
src/test/python/apache/thermos/common/test_planner.py::test_planner_unsatisfiables
 PASSED
 
  FAILURES 
 _ test_environment_names _
 
 def test_environment_names():
   base_job = Job(
   name='hello_world', role='john_doe', 
cluster='test-cluster',
   task=Task(name='main', processes=[],
 resources=Resources(cpu=0.1, 
ram=64 * MB, disk=64 * MB)))
 
   with pytest.raises(ValueError):
 >   
config._validate_environment_name(AuroraConfig(base_job))
 E   AttributeError: 'module' object has no 
attribute '_validate_environment_name'
 
 src/test/python/apache/aurora/client/test_config.py:155: 
AttributeError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/aaf4d108c31293299a0839bdc404a91802f80937.xml
 
  1 failed, 795 passed, 6 skipped, 1 warnings in 
301.91 seconds 
 
FAILURE


15:15:36 05:43   [complete]
   FAILURE


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

- Aurora ReviewBot


On Oct. 14, 2017, 2:55 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 14, 2017, 2:55 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
>   docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/auro

Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-14 Thread Mauricio Garavaglia


> On Oct. 12, 2017, 8:08 p.m., Stephan Erb wrote:
> > Looks good to me in general.
> > 
> > We'll need to update the docs as well. Please:
> > 
> > * Add a section to the RELEASE-NOTES to indicate that scheduler 
> > environments are now configurable on the scheduler side
> > * Update the docs. In particular 
> > https://github.com/apache/aurora/blob/master/docs/features/multitenancy.md#job-namespaces
> >  and 
> > https://github.com/apache/aurora/blob/master/docs/reference/configuration.md#job-objects
> >  should be updated that these are just the defaults but could be changed by 
> > an operator
> > 
> > Thanks!

docs updated


> On Oct. 12, 2017, 8:08 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
> > Lines 70 (patched)
> > 
> >
> > IIRC, the OR operator has the lowest precedence. This would imply we'd 
> > need to change this regex to `^(prod|devel|test|staging\d*)$`

good catch!


- Mauricio


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


On Oct. 14, 2017, 2:55 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 14, 2017, 2:55 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
>   docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 54d7d4ce0e93ca2278bb5f176e633ad72991ee9d 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 60bbe39d1282ccaa1bb91ad595412d55d115e6c5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b7f5e35079431d26065bc95430badb3e6d229a2a 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  265e87ea9ec0bf3a73a1b5e9d7c40e3d9c79e863 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 459d6bebcd7d6341dac2aead7e3dd8ce87bc9ed6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 2cd19d572d2945e28630b9bce5ba58eb9753630a 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-14 Thread Mauricio Garavaglia

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

(Updated Oct. 14, 2017, 2:55 p.m.)


Review request for Aurora and Stephan Erb.


Changes
---

rebased and fixed default regexp


Repository: aurora


Description
---

Moves the job environment validation to the scheduler, which can be enabled 
with the scheduler require_predefined_environments flag. This allows to have a 
consistent behavior when using the CLI and the API. In order to preserve 
backward compatibility, the validation is kept in the CLI and for the API it 
needs to be manually enabled in the scheduler.


Diffs (updated)
-

  docs/features/multitenancy.md 301170daa5ecb92f748994c40bb3f9f1f3871e53 
  docs/reference/configuration.md 0231c9265a8134e9b4541e131c3589c96df81274 
  docs/reference/scheduler-configuration.md 
4e3f90713c307e3b9e9f84c29343af7f014f0165 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
54d7d4ce0e93ca2278bb5f176e633ad72991ee9d 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
60bbe39d1282ccaa1bb91ad595412d55d115e6c5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b7f5e35079431d26065bc95430badb3e6d229a2a 
  src/main/python/apache/aurora/client/config.py 
70c2c980309e18de576b251087cdfea00ac06b75 
  src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 265e87ea9ec0bf3a73a1b5e9d7c40e3d9c79e863 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
459d6bebcd7d6341dac2aead7e3dd8ce87bc9ed6 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
3c5ecd698557cafdf8eeacdc472589a379018896 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
2cd19d572d2945e28630b9bce5ba58eb9753630a 


Diff: https://reviews.apache.org/r/62692/diff/5/

Changes: https://reviews.apache.org/r/62692/diff/4-5/


Testing
---


Thanks,

Mauricio Garavaglia



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-12 Thread Stephan Erb

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



Looks good to me in general.

We'll need to update the docs as well. Please:

* Add a section to the RELEASE-NOTES to indicate that scheduler environments 
are now configurable on the scheduler side
* Update the docs. In particular 
https://github.com/apache/aurora/blob/master/docs/features/multitenancy.md#job-namespaces
 and 
https://github.com/apache/aurora/blob/master/docs/reference/configuration.md#job-objects
 should be updated that these are just the defaults but could be changed by an 
operator

Thanks!


src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
Lines 70 (patched)


IIRC, the OR operator has the lowest precedence. This would imply we'd need 
to change this regex to `^(prod|devel|test|staging\d*)$`



src/main/python/apache/aurora/client/config.py
Lines 58-73 (original)


I am ok with this, but I am calling this out so that everyone is aware: 

With this change a new client will be able to create arbitrary environments 
on old scheduler instances.


- Stephan Erb


On Oct. 12, 2017, 8:20 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 12, 2017, 8:20 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 8556253fc11f6027316871eb9dc66d8627a77fe6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  9c446682750706aecdc67062ae82f2a76ab38043 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-12 Thread Renan DelValle

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


Fix it, then Ship it!





src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
Line 94 (original), 96 (patched)


@mauriciog Looks like Bill's recent commit 
(https://github.com/apache/aurora/commit/519e3df732ea5d009d92685ee3c4823606eb2462)
 causes this patch to not apply cleanly. A rebase off the master should do the 
trick. Otherwise, looks good to me.


- Renan DelValle


On Oct. 12, 2017, 11:20 a.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 12, 2017, 11:20 a.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 8556253fc11f6027316871eb9dc66d8627a77fe6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  9c446682750706aecdc67062ae82f2a76ab38043 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-12 Thread Mohit Jaggi

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


Ship it!




Ship It!

- Mohit Jaggi


On Oct. 12, 2017, 6:20 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 12, 2017, 6:20 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 8556253fc11f6027316871eb9dc66d8627a77fe6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  9c446682750706aecdc67062ae82f2a76ab38043 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-12 Thread Aurora ReviewBot

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



This patch does not apply cleanly against master (519e3df), do you need to 
rebase?

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

- Aurora ReviewBot


On Oct. 12, 2017, 11:20 a.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 12, 2017, 11:20 a.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   docs/reference/scheduler-configuration.md 
> 4e3f90713c307e3b9e9f84c29343af7f014f0165 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 8556253fc11f6027316871eb9dc66d8627a77fe6 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  9c446682750706aecdc67062ae82f2a76ab38043 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-12 Thread Mauricio Garavaglia

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

(Updated Oct. 12, 2017, 6:20 p.m.)


Review request for Aurora and Stephan Erb.


Changes
---

Implemented environment validation based on a given regexp


Repository: aurora


Description
---

Moves the job environment validation to the scheduler, which can be enabled 
with the scheduler require_predefined_environments flag. This allows to have a 
consistent behavior when using the CLI and the API. In order to preserve 
backward compatibility, the validation is kept in the CLI and for the API it 
needs to be manually enabled in the scheduler.


Diffs (updated)
-

  docs/reference/scheduler-configuration.md 
4e3f90713c307e3b9e9f84c29343af7f014f0165 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
081dff2bda626f01ba222628b8d7d8afebbb0004 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
186fa1b3a4780c0536fb486d50a33133258110cd 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 754fde0fdc976b673d78ae15d8ccd8c85b792373 
  src/main/python/apache/aurora/client/config.py 
70c2c980309e18de576b251087cdfea00ac06b75 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
8556253fc11f6027316871eb9dc66d8627a77fe6 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
3c5ecd698557cafdf8eeacdc472589a379018896 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 9c446682750706aecdc67062ae82f2a76ab38043 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
6b4b17f8dafd5c2d751dcda3080b476335f8aec0 


Diff: https://reviews.apache.org/r/62692/diff/4/

Changes: https://reviews.apache.org/r/62692/diff/3-4/


Testing
---


Thanks,

Mauricio Garavaglia



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-11 Thread Mohit Jaggi


> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```
> 
> Mauricio Garavaglia wrote:
> I'd expect \.* to be the default then, right?
> 
> Bill Farner wrote:
> If starting from scratch, yes.  In this case, it's more friendly to 
> prevent surprises by maintaining existing behavior by default.
> 
> Mauricio Garavaglia wrote:
> The existing behavior is that the scheduler API ignores whatever 
> environment comes in, but the CLI restricts it to the list there. Are you 
> suggesting that now the default, for the API, should be to only allow 
> 'prod,devel,test,^staging\\d+'? How is that maintaining existing behavior by 
> default?
> 
> Bill Farner wrote:
> My mistake!  I believed the scheduler was already restricting this, but i 
> stand corrected.  I support your suggestion for `".*"` as the default.
> 
> Stephan Erb wrote:
> I am not convinced we would need to enforce backwards compatibility here. 
> 
> Most Aurora users will use the default client and would thus not be 
> affected if we use the same enforcement at the scheduler side. For those 
> clusers with custom clients, I think it is OK to ask the operator to 
> add/update the scheduler flag when deploying the latest scheduler version. 
> Yes, it is a breaking change but at least one that is very easy to handle if 
> we call it out in the RELEASE-NOTES.md.
> 
> Mauricio Garavaglia wrote:
> Sure, the default can be kept the same. I still found the whole 
> environment validation extremely arbitrary, with no clear implication for the 
> cluster operator, and as such is not clear what's its purpose other than 
> preserve an old behavior.
> Anyway, regarding the validation format, is that something like a comma 
> separated list of Regexps? or is that just a Regexp?
> 
> Stephan Erb wrote:
> I believe having just one regex would be the most flexible: 
> `prod,devel,test,^staging\d+*` could be written as 
> `prod|devel|test|^staging\d+*`

A single regexp can do an OR of patterns, so we don't need a comma separated 
list. One might be tempted to attempt a more "user-friendly" format but regexp 
is a pretty standard construct and I would suggest not re-inventing the wheel. 
Yet, I don't have a strong opinion and would like to see this and 319 closed 
quickly :-)


- Mohit


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


On Oct. 1, 2017, 3:36 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 1, 2017, 3:36 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-11 Thread Stephan Erb


> On Sept. 29, 2017, 7:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```
> 
> Mauricio Garavaglia wrote:
> I'd expect \.* to be the default then, right?
> 
> Bill Farner wrote:
> If starting from scratch, yes.  In this case, it's more friendly to 
> prevent surprises by maintaining existing behavior by default.
> 
> Mauricio Garavaglia wrote:
> The existing behavior is that the scheduler API ignores whatever 
> environment comes in, but the CLI restricts it to the list there. Are you 
> suggesting that now the default, for the API, should be to only allow 
> 'prod,devel,test,^staging\\d+'? How is that maintaining existing behavior by 
> default?
> 
> Bill Farner wrote:
> My mistake!  I believed the scheduler was already restricting this, but i 
> stand corrected.  I support your suggestion for `".*"` as the default.
> 
> Stephan Erb wrote:
> I am not convinced we would need to enforce backwards compatibility here. 
> 
> Most Aurora users will use the default client and would thus not be 
> affected if we use the same enforcement at the scheduler side. For those 
> clusers with custom clients, I think it is OK to ask the operator to 
> add/update the scheduler flag when deploying the latest scheduler version. 
> Yes, it is a breaking change but at least one that is very easy to handle if 
> we call it out in the RELEASE-NOTES.md.
> 
> Mauricio Garavaglia wrote:
> Sure, the default can be kept the same. I still found the whole 
> environment validation extremely arbitrary, with no clear implication for the 
> cluster operator, and as such is not clear what's its purpose other than 
> preserve an old behavior.
> Anyway, regarding the validation format, is that something like a comma 
> separated list of Regexps? or is that just a Regexp?

I believe having just one regex would be the most flexible: 
`prod,devel,test,^staging\d+*` could be written as 
`prod|devel|test|^staging\d+*`


- Stephan


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


On Oct. 1, 2017, 5:36 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 1, 2017, 5:36 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-11 Thread Mauricio Garavaglia


> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```
> 
> Mauricio Garavaglia wrote:
> I'd expect \.* to be the default then, right?
> 
> Bill Farner wrote:
> If starting from scratch, yes.  In this case, it's more friendly to 
> prevent surprises by maintaining existing behavior by default.
> 
> Mauricio Garavaglia wrote:
> The existing behavior is that the scheduler API ignores whatever 
> environment comes in, but the CLI restricts it to the list there. Are you 
> suggesting that now the default, for the API, should be to only allow 
> 'prod,devel,test,^staging\\d+'? How is that maintaining existing behavior by 
> default?
> 
> Bill Farner wrote:
> My mistake!  I believed the scheduler was already restricting this, but i 
> stand corrected.  I support your suggestion for `".*"` as the default.
> 
> Stephan Erb wrote:
> I am not convinced we would need to enforce backwards compatibility here. 
> 
> Most Aurora users will use the default client and would thus not be 
> affected if we use the same enforcement at the scheduler side. For those 
> clusers with custom clients, I think it is OK to ask the operator to 
> add/update the scheduler flag when deploying the latest scheduler version. 
> Yes, it is a breaking change but at least one that is very easy to handle if 
> we call it out in the RELEASE-NOTES.md.

Sure, the default can be kept the same. I still found the whole environment 
validation extremely arbitrary, with no clear implication for the cluster 
operator, and as such is not clear what's its purpose other than preserve an 
old behavior.
Anyway, regarding the validation format, is that something like a comma 
separated list of Regexps? or is that just a Regexp?


- Mauricio


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


On Oct. 1, 2017, 3:36 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 1, 2017, 3:36 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-05 Thread Mohit Jaggi


> On Oct. 3, 2017, 9:49 p.m., Stephan Erb wrote:
> > Sorry for jumping in so late.
> > 
> > We have had discussions and some work on this previously (see 
> > https://issues.apache.org/jira/browse/AURORA-319). I do therefore think it 
> > makes sense to change the scheduler side to a generic regex-based 
> > validation (as also proposed by Bill). This will give operators full 
> > control over supported environment names. We could then merge 
> > https://reviews.apache.org/r/45521/ once this RB here has been merged and 
> > released.

Second this. I can help if needed.


- Mohit


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


On Oct. 1, 2017, 3:36 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 1, 2017, 3:36 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-03 Thread Stephan Erb


> On Sept. 29, 2017, 7:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```
> 
> Mauricio Garavaglia wrote:
> I'd expect \.* to be the default then, right?
> 
> Bill Farner wrote:
> If starting from scratch, yes.  In this case, it's more friendly to 
> prevent surprises by maintaining existing behavior by default.
> 
> Mauricio Garavaglia wrote:
> The existing behavior is that the scheduler API ignores whatever 
> environment comes in, but the CLI restricts it to the list there. Are you 
> suggesting that now the default, for the API, should be to only allow 
> 'prod,devel,test,^staging\\d+'? How is that maintaining existing behavior by 
> default?
> 
> Bill Farner wrote:
> My mistake!  I believed the scheduler was already restricting this, but i 
> stand corrected.  I support your suggestion for `".*"` as the default.

I am not convinced we would need to enforce backwards compatibility here. 

Most Aurora users will use the default client and would thus not be affected if 
we use the same enforcement at the scheduler side. For those clusers with 
custom clients, I think it is OK to ask the operator to add/update the 
scheduler flag when deploying the latest scheduler version. Yes, it is a 
breaking change but at least one that is very easy to handle if we call it out 
in the RELEASE-NOTES.md.


- Stephan


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


On Oct. 1, 2017, 5:36 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 1, 2017, 5:36 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-03 Thread Stephan Erb

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



Sorry for jumping in so late.

We have had discussions and some work on this previously (see 
https://issues.apache.org/jira/browse/AURORA-319). I do therefore think it 
makes sense to change the scheduler side to a generic regex-based validation 
(as also proposed by Bill). This will give operators full control over 
supported environment names. We could then merge 
https://reviews.apache.org/r/45521/ once this RB here has been merged and 
released.

- Stephan Erb


On Oct. 1, 2017, 5:36 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 1, 2017, 5:36 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-01 Thread Aurora ReviewBot

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


Ship it!




Master (24d2caf) 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 Oct. 1, 2017, 8:36 a.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Oct. 1, 2017, 8:36 a.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-01 Thread Mauricio Garavaglia

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

(Updated Oct. 1, 2017, 3:36 p.m.)


Review request for Aurora and Stephan Erb.


Repository: aurora


Description
---

Moves the job environment validation to the scheduler, which can be enabled 
with the scheduler require_predefined_environments flag. This allows to have a 
consistent behavior when using the CLI and the API. In order to preserve 
backward compatibility, the validation is kept in the CLI and for the API it 
needs to be manually enabled in the scheduler.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
081dff2bda626f01ba222628b8d7d8afebbb0004 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
186fa1b3a4780c0536fb486d50a33133258110cd 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 754fde0fdc976b673d78ae15d8ccd8c85b792373 
  src/main/python/apache/aurora/client/config.py 
70c2c980309e18de576b251087cdfea00ac06b75 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
6b4b17f8dafd5c2d751dcda3080b476335f8aec0 


Diff: https://reviews.apache.org/r/62692/diff/3/

Changes: https://reviews.apache.org/r/62692/diff/2-3/


Testing
---


Thanks,

Mauricio Garavaglia



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-01 Thread David McLaughlin

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


Ship it!




+1 for thin client/fat server. Thanks for the patch!

Ship It contingent on a green ReviewBot result.

- David McLaughlin


On Sept. 30, 2017, 3:04 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 30, 2017, 3:04 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-30 Thread Bill Farner


> On Sept. 29, 2017, 10:38 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```
> 
> Mauricio Garavaglia wrote:
> I'd expect \.* to be the default then, right?
> 
> Bill Farner wrote:
> If starting from scratch, yes.  In this case, it's more friendly to 
> prevent surprises by maintaining existing behavior by default.
> 
> Mauricio Garavaglia wrote:
> The existing behavior is that the scheduler API ignores whatever 
> environment comes in, but the CLI restricts it to the list there. Are you 
> suggesting that now the default, for the API, should be to only allow 
> 'prod,devel,test,^staging\\d+'? How is that maintaining existing behavior by 
> default?

My mistake!  I believed the scheduler was already restricting this, but i stand 
corrected.  I support your suggestion for `".*"` as the default.


- Bill


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


On Sept. 30, 2017, 8:04 a.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 30, 2017, 8:04 a.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-30 Thread Aurora ReviewBot

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



Master (24d2caf) is red with this patch.
  ./build-support/jenkins/build.sh

  Running setup.py bdist_wheel for twitter.common.log: finished with status 
'done'
  Stored in directory: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pip/wheels/80/e3/3a/297d8950fcbd822ab5a6a62175818fab38b668cc5a86dbd0b0
  Running setup.py bdist_wheel for pycparser: started
  Running setup.py bdist_wheel for pycparser: finished with status 'done'
  Stored in directory: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pip/wheels/95/14/9a/5e7b9024459d2a6600aaa64e0ba485325aff7a9ac7489db1b6
  Running setup.py bdist_wheel for twitter.common.options: started
  Running setup.py bdist_wheel for twitter.common.options: finished with status 
'done'
  Stored in directory: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pip/wheels/17/41/80/c4811d8c1c7ca7007e520c3399872fc340f45c3a26a6a23e6a
Successfully built pantsbuild.pants twitter.common.collections setproctitle 
ansicolors pathspec scandir twitter.common.dirutil psutil pystache docutils 
Markdown Pygments twitter.common.confluence coverage lmdb pywatchman 
twitter.common.lang twitter.common.log pycparser twitter.common.options
Installing collected packages: twitter.common.lang, twitter.common.collections, 
setproctitle, setuptools, six, ansicolors, pyparsing, packaging, pathspec, 
scandir, twitter.common.dirutil, psutil, requests, pystache, pex, docutils, 
Markdown, Pygments, twitter.common.options, twitter.common.log, 
twitter.common.confluence, monotonic, fasteners, coverage, pycparser, cffi, 
lmdb, pywatchman, futures, pantsbuild.pants
  Found existing installation: setuptools 21.2.1
Uninstalling setuptools-21.2.1:
  Successfully uninstalled setuptools-21.2.1
Successfully installed Markdown-2.1.1 Pygments-1.4 ansicolors-1.0.2 cffi-1.7.0 
coverage-3.7.1 docutils-0.12 fasteners-0.14.1 futures-3.0.5 lmdb-0.89 
monotonic-1.3 packaging-16.7 pantsbuild.pants-1.3.0.dev3 pathspec-0.3.4 
pex-1.1.16 psutil-4.3.0 pycparser-2.18 pyparsing-2.2.0 pystache-0.5.3 
pywatchman-1.3.0 requests-2.5.3 scandir-1.2 setproctitle-1.1.10 
setuptools-30.0.0 six-1.11.0 twitter.common.collections-0.3.9 
twitter.common.confluence-0.3.9 twitter.common.dirutil-0.3.9 
twitter.common.lang-0.3.9 twitter.common.log-0.3.9 twitter.common.options-0.3.9
You are using pip version 8.1.2, however version 9.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

15:18:51 00:00 [main]
   (To run a reporting server: ./pants server)
15:18:51 00:00   [setup]
15:18:51 00:00 [parse]
   Executing tasks in goals: compile
15:18:51 00:00   [compile]
15:18:51 00:00 [compile-prep-command]
15:18:51 00:00   [prep_command]
15:18:54 00:03 [compile]
15:18:54 00:03 [python-eval]
15:18:54 00:03 [pythonstyle]
15:18:54 00:03   [cache]  
   No cached artifacts for 42 targets.
   Invalidated 42 targets.
E265:ERROR   PythonFile(src/main/python/apache/aurora/client/config.py):149 
block comment should start with '# '
 |  #TODO Deprecate the environment validation once it can be enabled in 
the scheduler.


FAILURE: 1 Python Style issues found. For import order related issues, please 
try `./pants fmt.isort `


15:19:17 00:26   [complete]
   FAILURE


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

- Aurora ReviewBot


On Sept. 30, 2017, 3:04 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 30, 2017, 3:04 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config

Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-30 Thread Mauricio Garavaglia

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

(Updated Sept. 30, 2017, 3:04 p.m.)


Review request for Aurora and Stephan Erb.


Changes
---

Fixed code style


Repository: aurora


Description
---

Moves the job environment validation to the scheduler, which can be enabled 
with the scheduler require_predefined_environments flag. This allows to have a 
consistent behavior when using the CLI and the API. In order to preserve 
backward compatibility, the validation is kept in the CLI and for the API it 
needs to be manually enabled in the scheduler.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
081dff2bda626f01ba222628b8d7d8afebbb0004 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
186fa1b3a4780c0536fb486d50a33133258110cd 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 754fde0fdc976b673d78ae15d8ccd8c85b792373 
  src/main/python/apache/aurora/client/config.py 
70c2c980309e18de576b251087cdfea00ac06b75 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
6b4b17f8dafd5c2d751dcda3080b476335f8aec0 


Diff: https://reviews.apache.org/r/62692/diff/2/

Changes: https://reviews.apache.org/r/62692/diff/1-2/


Testing
---


Thanks,

Mauricio Garavaglia



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-30 Thread Mauricio Garavaglia


> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```
> 
> Mauricio Garavaglia wrote:
> I'd expect \.* to be the default then, right?
> 
> Bill Farner wrote:
> If starting from scratch, yes.  In this case, it's more friendly to 
> prevent surprises by maintaining existing behavior by default.

The existing behavior is that the scheduler API ignores whatever environment 
comes in, but the CLI restricts it to the list there. Are you suggesting that 
now the default, for the API, should be to only allow 
'prod,devel,test,^staging\\d+'? How is that maintaining existing behavior by 
default?


- Mauricio


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


On Sept. 29, 2017, 4:44 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 29, 2017, 4:44 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-29 Thread Bill Farner


> On Sept. 29, 2017, 10:38 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```
> 
> Mauricio Garavaglia wrote:
> I'd expect \.* to be the default then, right?

If starting from scratch, yes.  In this case, it's more friendly to prevent 
surprises by maintaining existing behavior by default.


- Bill


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


On Sept. 29, 2017, 9:44 a.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 29, 2017, 9:44 a.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-29 Thread Mauricio Garavaglia


> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > 
> >
> > I would find this easier to read and more flexible:
> > 
> > ```
> > -allowed_job_environments=prod,devel,test,^staging\d+*
> > ```
> > 
> > Then you can do:
> > ```
> > -allowed_job_environments=.*
> > ```

I'd expect \.* to be the default then, right?


- Mauricio


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


On Sept. 29, 2017, 4:44 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 29, 2017, 4:44 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-29 Thread Bill Farner

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




src/main/java/org/apache/aurora/scheduler/app/AppModule.java
Lines 108 (patched)


I would find this easier to read and more flexible:

```
-allowed_job_environments=prod,devel,test,^staging\d+*
```

Then you can do:
```
-allowed_job_environments=.*
```


- Bill Farner


On Sept. 29, 2017, 9:44 a.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 29, 2017, 9:44 a.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>



Re: Review Request 62692: Move job environment validation to the scheduler.

2017-09-29 Thread Aurora ReviewBot

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



Master (7a80373) is red with this patch.
  ./build-support/jenkins/build.sh

:startScripts
:distTar
:distZip
:assemble
:compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java:38:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.thrift.aop.MockDecoratedThriftForwarder
@Forward(AnnotatedAuroraAdmin.class)
^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processTestResources
:testClasses
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources NO-SOURCE
:jmhClasses
:checkstyleJmh
:checkstyleMain[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/app/AppModule.java:108:
 Line has trailing spaces. [RegexpSingleline]
[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/app/AppModule.java:108:
 Line is longer than 100 characters (found 102). [LineLength]
[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/app/AppModule.java:110:71:
 '=' is not preceded with whitespace. [WhitespaceAround]
[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java:191:
 Line is longer than 100 characters (found 105). [LineLength]
[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java:212:
 Line is longer than 100 characters (found 113). [LineLength]
[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java:212:17:
 'static' modifier out of order with the JLS suggestions. [ModifierOrder]
[ant:checkstyle] [ERROR] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java:213:17:
 'static' modifier out of order with the JLS suggestions. [ModifierOrder]
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

* Get more help at https://help.gradle.org

BUILD FAILED in 3m 37s
37 actionable tasks: 31 executed, 6 up-to-date


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

- Aurora ReviewBot


On Sept. 29, 2017, 4:44 p.m., Mauricio Garavaglia wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62692/
> ---
> 
> (Updated Sept. 29, 2017, 4:44 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Moves the job environment validation to the scheduler, which can be enabled 
> with the scheduler require_predefined_environments flag. This allows to have 
> a consistent behavior when using the CLI and the API. In order to preserve 
> backward compatibility, the validation is kept in the CLI and for the API it 
> needs to be manually enabled in the scheduler.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 081dff2bda626f01ba222628b8d7d8afebbb0004 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/python/apache/aurora/client/config.py 
> 70c2c980309e18de576b251087cdfea00ac06b75 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 6b4b17f8dafd5c2d751dcda3080b476335f8aec0 
> 
> 
> Diff: https://reviews.apache.org/r/62692/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mauricio Garavaglia
> 
>