You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Mauricio Garavaglia <ma...@gmail.com> on 2017/09/29 16:44:37 UTC
Review Request 62692: Move job environment validation to the
scheduler.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62692/
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 62692: Move job environment validation to the
scheduler.
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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][32m
Executing tasks in goals: compile[0m
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] [32m
No cached artifacts for 42 targets.[0m[32m
Invalidated 42 targets.[0m
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 <targets>`
15:19:17 00:26 [complete][31m
FAILURE[0m
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.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.
Posted by Mohit Jaggi <mo...@gmail.com>.
> 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.
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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 [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_non_ephemeral_process_cannot_depend_on_ephemeral_process [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_failed_predecessor_does_not_make_process_runnable [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_daemon_duration [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_waits [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_fails [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_lost [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_filters [32mPASSED[0m
src/test/python/apache/thermos/common/test_task_planner.py::test_task_max_runs [32mPASSED[0m
src/test/python/apache/thermos/common/test_pathspec.py::test_legacy_task_roots [32mPASSED[0m
src/test/python/apache/thermos/common/test_pathspec.py::test_legacy_log_dirs [32mPASSED[0m
src/test/python/apache/thermos/common/test_pathspec.py::test_exception_on_none_keys [32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_empty [32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_unordered [32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_ordered [32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_mixed [32mPASSED[0m
src/test/python/apache/thermos/common/test_planner.py::test_planner_unsatisfiables [32mPASSED[0m
==================== FAILURES ====================
_____________ test_environment_names _____________
[1m def test_environment_names():[0m
[1m base_job = Job([0m
[1m name='hello_world', role='john_doe', cluster='test-cluster',[0m
[1m task=Task(name='main', processes=[],[0m
[1m resources=Resources(cpu=0.1, ram=64 * MB, disk=64 * MB)))[0m
[1m [0m
[1m with pytest.raises(ValueError):[0m
[1m> config._validate_environment_name(AuroraConfig(base_job))[0m
[1m[31mE AttributeError: 'module' object has no attribute '_validate_environment_name'[0m
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
[1m[31m 1 failed, 795 passed, 6 skipped, 1 warnings in 301.91 seconds [0m
FAILURE
15:15:36 05:43 [complete][31m
FAILURE[0m
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/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.
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.
Posted by Mohit Jaggi <mo...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
> 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)
> > <https://reviews.apache.org/r/62692/diff/4/?file=1853684#file1853684line70>
> >
> > 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.
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/62692/#comment264891>
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)
<https://reviews.apache.org/r/62692/#comment264892>
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.
Posted by Renan DelValle <re...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/62692/#comment264879>
@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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
-----------------------------------------------------------
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.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 29, 2017, 10:38 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 29, 2017, 10:38 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Stephan Erb <se...@apache.org>.
> On Sept. 29, 2017, 7:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Mohit Jaggi <mo...@gmail.com>.
> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Stephan Erb <se...@apache.org>.
> On Sept. 29, 2017, 7:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Mauricio Garavaglia <ma...@gmail.com>.
> On Sept. 29, 2017, 5:38 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/62692/diff/1/?file=1839853#file1839853line108>
> >
> > 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.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/62692/#comment263497>
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
>
>