Review Request 32295: Make health check configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32295/ --- Review request for Aurora. Bugs: AURORA-316 https://issues.apache.org/jira/browse/AURORA-316 Repository: aurora Description --- This allows the endpoint, expected response and expected response code to be configured by the user. Diffs - docs/configuration-reference.md af332f2 src/main/python/apache/aurora/common/http_signaler.py e3e819d src/main/python/apache/aurora/config/schema/base.py a87524a src/main/python/apache/aurora/executor/common/health_checker.py cfc29c3 src/test/python/apache/aurora/common/test_http_signaler.py f5f8419 src/test/python/apache/aurora/executor/common/test_health_checker.py 1b4423a Diff: https://reviews.apache.org/r/32295/diff/ Testing --- Unittests added and manurally verified in vagrant enviroment. Thanks, Brian Brazil
Re: Review Request 32295: Make health check configurable
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32295/#review77198
---
Master (db79418) is red with this patch.
./build-support/jenkins/build.sh
Installing
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.app-0.3.0-py2.7-nspkg.pth
Running setup.py install for GitPython
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/local/lib/python2.7/site-packages/setuptools/dist.py:292:
UserWarning: The version specified ('0.3.2 RC1') is an invalid version, this
may not work as expected with newer versions of setuptools, pip, and PyPI.
Please see PEP 440 for more details.
details. % self.metadata.version
Running setup.py install for pep8
Installing pep8 script to
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
Running setup.py install for pyflakes
Installing pyflakes script to
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
Running setup.py install for twitter.checkstyle
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
(namespace package)
Installing
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.checkstyle-0.1.0-py2.7-nspkg.pth
Installing twitterstyle script to
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
Successfully installed GitPython-0.3.2rc1 gitdb-0.6.4 pep8-1.4.5 pyflakes-0.7.2
smmap-0.9.0 twitter.checkstyle-0.1.0 twitter.common.app-0.3.0
twitter.common.collections-0.3.0 twitter.common.contextutil-0.3.0
twitter.common.dirutil-0.3.0 twitter.common.lang-0.3.0 twitter.common.log-0.3.0
twitter.common.options-0.3.0 twitter.common.process-0.3.0
twitter.common.string-0.3.0 twitter.common.util-0.3.0
E501:ERROR src/main/python/apache/aurora/common/http_signaler.py:085 line too
long (107 100 characters)
| def __call__(self, endpoint, use_post_method=False,
expected_response=None, expected_response_code=None):
E303:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:066 too
many blank lines (2)
| def test_health_checks(self):
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:073
line too long (107 100 characters)
| 'http://localhost:%s/health' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('not ok', code=200))
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:075
line too long (103 100 characters)
| 'http://localhost:%s/health' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('ok', code=400))
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:080
line too long (103 100 characters)
| 'http://localhost:%s/health' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('ok', code=200))
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:082
line too long (102 100 characters)
| 'http://localhost:%s/random/endpoint' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('ok'))
T302:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:028
Expected 2 blank lines, found 1
|class OpenedURL(object):
T302:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:042
Expected 2 blank lines, found 1
|class TestHttpSignaler(unittest.TestCase):
T301:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:066
Expected 1 blank lines, found 2
| def test_health_checks(self):
I will refresh this build result if you post a review containing @ReviewBot
retry
- Aurora ReviewBot
On March 20, 2015, 10:57 a.m., Brian Brazil wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32295/
---
(Updated March 20, 2015, 10:57 a.m.)
Review request for Aurora.
Bugs: AURORA-316
https://issues.apache.org/jira/browse/AURORA-316
Repository: aurora
Description
---
This allows the endpoint, expected response
and expected response code to be configured by the user.
Diffs
-
docs/configuration-reference.md af332f2
src/main/python/apache/aurora/common/http_signaler.py e3e819d
src/main/python/apache/aurora/config/schema/base.py a87524a
src/main/python/apache/aurora/executor/common/health_checker.py cfc29c3
src/test/python/apache/aurora/common/test_http_signaler.py f5f8419
src/test/python/apache/aurora/executor/common/test_health_checker.py
1b4423a
Diff:
Re: Review Request 32295: Make health check configurable
On March 20, 2015, 11:06 a.m., Aurora ReviewBot wrote:
Master (db79418) is red with this patch.
./build-support/jenkins/build.sh
Installing
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.app-0.3.0-py2.7-nspkg.pth
Running setup.py install for GitPython
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/local/lib/python2.7/site-packages/setuptools/dist.py:292:
UserWarning: The version specified ('0.3.2 RC1') is an invalid version,
this may not work as expected with newer versions of setuptools, pip, and
PyPI. Please see PEP 440 for more details.
details. % self.metadata.version
Running setup.py install for pep8
Installing pep8 script to
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
Running setup.py install for pyflakes
Installing pyflakes script to
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
Running setup.py install for twitter.checkstyle
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
(namespace package)
Installing
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.checkstyle-0.1.0-py2.7-nspkg.pth
Installing twitterstyle script to
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/bin
Successfully installed GitPython-0.3.2rc1 gitdb-0.6.4 pep8-1.4.5
pyflakes-0.7.2 smmap-0.9.0 twitter.checkstyle-0.1.0
twitter.common.app-0.3.0 twitter.common.collections-0.3.0
twitter.common.contextutil-0.3.0 twitter.common.dirutil-0.3.0
twitter.common.lang-0.3.0 twitter.common.log-0.3.0
twitter.common.options-0.3.0 twitter.common.process-0.3.0
twitter.common.string-0.3.0 twitter.common.util-0.3.0
E501:ERROR src/main/python/apache/aurora/common/http_signaler.py:085 line
too long (107 100 characters)
| def __call__(self, endpoint, use_post_method=False,
expected_response=None, expected_response_code=None):
E303:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:066
too many blank lines (2)
| def test_health_checks(self):
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:073
line too long (107 100 characters)
| 'http://localhost:%s/health' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('not ok', code=200))
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:075
line too long (103 100 characters)
| 'http://localhost:%s/health' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('ok', code=400))
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:080
line too long (103 100 characters)
| 'http://localhost:%s/health' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('ok', code=200))
E501:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:082
line too long (102 100 characters)
| 'http://localhost:%s/random/endpoint' % self.PORT, None,
timeout=1.0).AndReturn(OpenedURL('ok'))
T302:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:028
Expected 2 blank lines, found 1
|class OpenedURL(object):
T302:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:042
Expected 2 blank lines, found 1
|class TestHttpSignaler(unittest.TestCase):
T301:ERROR src/test/python/apache/aurora/common/test_http_signaler.py:066
Expected 1 blank lines, found 2
| def test_health_checks(self):
I will refresh this build result if you post a review containing
@ReviewBot retry
@ReviewBot retry
- Brian
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32295/#review77198
---
On March 20, 2015, 11:13 a.m., Brian Brazil wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32295/
---
(Updated March 20, 2015, 11:13 a.m.)
Review request for Aurora.
Bugs: AURORA-316
https://issues.apache.org/jira/browse/AURORA-316
Repository: aurora
Description
---
This allows the endpoint, expected response
and expected response code to be configured by the user.
Diffs
-
docs/configuration-reference.md af332f2
src/main/python/apache/aurora/common/http_signaler.py e3e819d
src/main/python/apache/aurora/config/schema/base.py a87524a
Re: Review Request 32295: Make health check configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32295/ --- (Updated March 20, 2015, 11:13 a.m.) Review request for Aurora. Bugs: AURORA-316 https://issues.apache.org/jira/browse/AURORA-316 Repository: aurora Description --- This allows the endpoint, expected response and expected response code to be configured by the user. Diffs (updated) - docs/configuration-reference.md af332f2 src/main/python/apache/aurora/common/http_signaler.py e3e819d src/main/python/apache/aurora/config/schema/base.py a87524a src/main/python/apache/aurora/executor/common/health_checker.py cfc29c3 src/test/python/apache/aurora/common/test_http_signaler.py f5f8419 src/test/python/apache/aurora/executor/common/test_health_checker.py 1b4423a Diff: https://reviews.apache.org/r/32295/diff/ Testing --- Unittests added and manurally verified in vagrant enviroment. Thanks, Brian Brazil
Re: Review Request 32295: Make health check configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32295/#review77203 --- Ship it! Master (db79418) 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 March 20, 2015, 11:13 a.m., Brian Brazil wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32295/ --- (Updated March 20, 2015, 11:13 a.m.) Review request for Aurora. Bugs: AURORA-316 https://issues.apache.org/jira/browse/AURORA-316 Repository: aurora Description --- This allows the endpoint, expected response and expected response code to be configured by the user. Diffs - docs/configuration-reference.md af332f2 src/main/python/apache/aurora/common/http_signaler.py e3e819d src/main/python/apache/aurora/config/schema/base.py a87524a src/main/python/apache/aurora/executor/common/health_checker.py cfc29c3 src/test/python/apache/aurora/common/test_http_signaler.py f5f8419 src/test/python/apache/aurora/executor/common/test_health_checker.py 1b4423a Diff: https://reviews.apache.org/r/32295/diff/ Testing --- Unittests added and manurally verified in vagrant enviroment. Thanks, Brian Brazil
Re: Review Request 32221: Remove excessively low timeout in SIGTERM swallowing test.
On March 20, 2015, 3:52 p.m., Joe Smith wrote: It seems like the `self.quitquitquit` is the important part (on line 340 of the runner)- doesn't decreasing the timeout not give `quitquitquit` the time it needs? - Joe --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32221/#review77296 --- On March 18, 2015, 6:20 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32221/ --- (Updated March 18, 2015, 6:20 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1054 https://issues.apache.org/jira/browse/AURORA-1054 Repository: aurora Description --- Remove excessively low timeout in SIGTERM swallowing test. Diffs - src/test/python/apache/aurora/executor/test_thermos_task_runner.py 6b24bbb2ab7ca16f97961aabeed945b61e5b5908 Diff: https://reviews.apache.org/r/32221/diff/ Testing --- Cannot reproduce locally, but 5 seconds is an impossibly small timeout, even if we aren't testing SIGTERM swallowing. If this fails, we will get tripped by 60s timeout instead. Thanks, Brian Wickman
Re: Review Request 32221: Remove excessively low timeout in SIGTERM swallowing test.
On March 20, 2015, 3:52 p.m., Joe Smith wrote:
Joe Smith wrote:
It seems like the `self.quitquitquit` is the important part (on line 340
of the runner)- doesn't decreasing the timeout not give `quitquitquit` the
time it needs?
In `src/main/python/apache/aurora/executor/thermos_task_runner.py`
```
331 waited = Amount(0, Time.SECONDS)
332 while self.is_alive and waited timeout:
333 self._clock.sleep(self.POLL_INTERVAL.as_(Time.SECONDS))
334 waited += self.POLL_INTERVAL
335
336 if not self.is_alive and self.task_state() != TaskState.ACTIVE:
337 return
338
339 log.info('Thermos task did not shut down cleanly, rebinding to kill.')
340 self.quitquitquit()
341
342 while not self._monitor.finished and waited timeout:
343 self._clock.sleep(self.POLL_INTERVAL.as_(Time.SECONDS))
344 waited += self.POLL_INTERVAL
```
Is it that we need to reset waited to Amount(0, Time.SECONDS) ?
- Joe
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32221/#review77296
---
On March 18, 2015, 6:20 p.m., Brian Wickman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32221/
---
(Updated March 18, 2015, 6:20 p.m.)
Review request for Aurora and Bill Farner.
Bugs: AURORA-1054
https://issues.apache.org/jira/browse/AURORA-1054
Repository: aurora
Description
---
Remove excessively low timeout in SIGTERM swallowing test.
Diffs
-
src/test/python/apache/aurora/executor/test_thermos_task_runner.py
6b24bbb2ab7ca16f97961aabeed945b61e5b5908
Diff: https://reviews.apache.org/r/32221/diff/
Testing
---
Cannot reproduce locally, but 5 seconds is an impossibly small timeout, even
if we aren't testing SIGTERM swallowing. If this fails, we will get tripped
by 60s timeout instead.
Thanks,
Brian Wickman
Re: Review Request 32319: Add a deprecation warning when using the client-side updater.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/#review77314 --- Ship it! The updater is dead, long live the updater. - Joshua Cohen On March 20, 2015, 10:27 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/ --- (Updated March 20, 2015, 10:27 p.m.) Review request for Aurora and Joshua Cohen. Bugs: AURORA-1190 https://issues.apache.org/jira/browse/AURORA-1190 Repository: aurora Description --- See summary. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 61337b9312864921508428c7f7992576b94a5d2c src/test/python/apache/aurora/client/cli/test_update.py cc7458568de121dc02de010687fcae0b2707ee0a src/test/python/apache/aurora/client/cli/util.py 864a71428f58ad5ea23beb1d9ae7520c82d2d276 Diff: https://reviews.apache.org/r/32319/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- (Updated March 20, 2015, 5:16 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Also the best place to start reviewing FieldGetterDag is also its tests and usage. Diffs (updated) - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java 4e341e05c34b1be38f0040c26b671a0cc797a771 src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5588d1793d6713ee4581ac9f938d9a8689acb315 src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java bdd2185f3a7a94b39bcec3c73455e970d87f0c6a src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java cafd10f6b705568588c1b92644b482003242fe2e src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java ed284f46ac8f01bd6d9e317f995f16d6e666a68d src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java d2ba2730c4509dc9a636fd32e9244b0d7fa2884f src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 1f24e7d47e1f777ffef19a73d01171fcacd31cdb src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b Diff: https://reviews.apache.org/r/32329/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/#review77319 --- Ship it! Master (a3a35e9) 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 March 21, 2015, 12:16 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- (Updated March 21, 2015, 12:16 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Also the best place to start reviewing FieldGetterDag is also its tests and usage. Diffs - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java 4e341e05c34b1be38f0040c26b671a0cc797a771 src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5588d1793d6713ee4581ac9f938d9a8689acb315 src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java bdd2185f3a7a94b39bcec3c73455e970d87f0c6a src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java cafd10f6b705568588c1b92644b482003242fe2e src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java ed284f46ac8f01bd6d9e317f995f16d6e666a68d src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java d2ba2730c4509dc9a636fd32e9244b0d7fa2884f src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 1f24e7d47e1f777ffef19a73d01171fcacd31cdb src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b Diff: https://reviews.apache.org/r/32329/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 30695: Implements log rotation in the Thermos runner.
On Feb. 6, 2015, 6:52 p.m., Brian Wickman wrote: This is super rad. Thanks for taking this on. Before I do a deeper dive, what do you think about making the logrotate policy be specified by the user instead of the framework owner, with a sensible default? For example, if this is configurable on the process object, you can have different policies per process, e.g. ```py class RotatePolicy(Struct): log_size = Default(Integer, 32*MB) backups = Default(Integer, 10) copytruncate = Default(Boolean, False) compress = Default(Boolean, False) hangup_command = String ... # union class Logger(Struct): standard = Boolean # standard i/o devnull = Boolean # /dev/null redirection logrotate = RotatePolicy # use a logrotation policy DefaultLogger = Logger(standard=True) class Process(Struct): cmdline = Required(String) name= Required(String) ... logger = Default(Logger, DefaultLogger) ``` This also means reduced end-to-end plumbing through all the binaries, class constructors, etc. And if you ever need to add new features (e.g. a compress option), they're fairly well encapsulated within the Logger union. George Sirois wrote: Awesome, thanks for the feedback. I'd be willing to take this on; it would definitely make the plumbing a lot cleaner and provide more flexibility, although the downside is that it's now harder to apply a universal default (besides whatever we arrive at as the Aurora default). I'll be able to pick this up next week and can probably have a modified review out by Wednesday evening. What do you think about starting out with a simple configuration (just log_size and backups on RotatePolicy) and iterating from there? I also have one question - what distinction are you making between the standard flag on Logger and the existence of a rotation policy? Brian Wickman wrote: Yeah, all the extra parameters were just for illustration only. Not asking for any more functionality than what you already have since it already provides tremendous value. The idea for 'standard' in Logger is just to be explicit about current behavior (unrestricted logging to stdout/stderr) and use it as the default. As for applying a universal default that's not standard, there are a few ways that you could do this, from environment variables (THERMOS_FORCE_ROTATE? idk) to building an aurora client using a custom entry point that patches Process.TYPEMAP['logger'] to use a different default. Both are kind of sketch but within the realm of sketch found elsewhere in the code. George Sirois wrote: The 'standard' flag makes sense to me, thanks. What do you envision reading the environment variable? The executor/runner? I suppose we could enhance the scheduler so that you can pass it environment variables to set when launching the executor so there wouldn't be a lot of plumbing. I guess in general I'm not a huge fan of using the client to enforce basic operational parameters like this (although I guess it's debatable as to whether or not these settings qualify :)). For example, it makes it much more challenging to move to a model where jobs are created/started through native API calls to the scheduler instead of using the client. Brian Wickman wrote: Sorry, I totally missed this follow-up comment. If you want to enforce defaults with the client out of the picture, then probably the best way to do this is to still implement the plumbing as described above but omit Default(Logger, DefaultLogger), letting it be Empty by default. Add command line parameters to thermos_runner that allow you to toggle which logger is the default (e.g. --process_logger='rotate' --rotate_log_size=...) With this in place, you can create a new TaskRunnerProvider (TellApartThermosTaskRunnerProvider? :-) or add flags to the default one that get plumbed through to the aurora_executor command line. (e.g. --default_process_logger=rotate) This at least allows you to set organization-wide policy and will be future-proof if/when the client goes the way of the dodo in favor of a REST API. Awesome, thanks. My preference is just for an extra flag to keep things simpler on our end for now. - George --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30695/#review71472 --- On Feb. 6, 2015, 5:51 p.m., George Sirois wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30695/ --- (Updated Feb. 6, 2015, 5:51 p.m.)
Re: Review Request 32353: Renaming PreemptionSlotFinder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/#review77325 --- Ship it! Master (a3a35e9) 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 March 21, 2015, 12:25 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/ --- (Updated March 21, 2015, 12:25 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- IDE-driven renaming. Prerequisite for the final preemptor refactoring step. Diffs - src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 Diff: https://reviews.apache.org/r/32353/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32353: Renaming PreemptionSlotFinder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/#review77317 --- Ship it! Ship It! - Bill Farner On March 21, 2015, 12:25 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/ --- (Updated March 21, 2015, 12:25 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- IDE-driven renaming. Prerequisite for the final preemptor refactoring step. Diffs - src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 Diff: https://reviews.apache.org/r/32353/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77302 --- Ship it! Master (f12d9fe) 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 March 20, 2015, 10:48 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:48 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Review Request 32353: Renaming PreemptionSlotFinder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- IDE-driven renaming. Prerequisite for the final preemptor refactoring step. Diffs - src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 Diff: https://reviews.apache.org/r/32353/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 32359: Adding a configurable delay into writing a backup file.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-1211 https://issues.apache.org/jira/browse/AURORA-1211 Repository: aurora Description --- Adding a configurable delay into writing a backup file. Diffs - src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java a6b187d888726b921733a36fcaecdab97bdef094 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 569648aea2ccdb57663d16b7c837fd2677694419 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 5853c037d53e707e5df434fc661cd285ed9ecfc4 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java ebe551739fb6b7132ce666ad9b3c5a86e90a5363 Diff: https://reviews.apache.org/r/32359/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32352: Making preemptor asynchronous. Part 3(final) - background service.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/#review77329 --- Ship it! Master (a3a35e9) 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 March 21, 2015, 2:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/ --- (Updated March 21, 2015, 2:19 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Summary of changes: - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction. - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots. - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots. - preemption_delay is reduced from 10 to 3 minutes. Benchmark refactoring will be addressed separately. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 Diff: https://reviews.apache.org/r/32352/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32221: Remove excessively low timeout in SIGTERM swallowing test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32221/#review77296 --- src/test/python/apache/aurora/executor/test_thermos_task_runner.py https://reviews.apache.org/r/32221/#comment125192 Maybe also decrease ``` 366poll_interval=Amount(500, Time.MILLISECONDS), ``` ? - Joe Smith On March 18, 2015, 6:20 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32221/ --- (Updated March 18, 2015, 6:20 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1054 https://issues.apache.org/jira/browse/AURORA-1054 Repository: aurora Description --- Remove excessively low timeout in SIGTERM swallowing test. Diffs - src/test/python/apache/aurora/executor/test_thermos_task_runner.py 6b24bbb2ab7ca16f97961aabeed945b61e5b5908 Diff: https://reviews.apache.org/r/32221/diff/ Testing --- Cannot reproduce locally, but 5 seconds is an impossibly small timeout, even if we aren't testing SIGTERM swallowing. If this fails, we will get tripped by 60s timeout instead. Thanks, Brian Wickman
Review Request 32352: Making preemptor asynchronous. Part 3(final) - background service.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/ --- Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Summary of changes: - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction. - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots. - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots. - preemption_delay is reduced from 10 to 3 minutes. Benchmark refactoring will be addressed separately. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 Diff: https://reviews.apache.org/r/32352/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32359: Adding a configurable delay into writing a backup file.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/#review77330 --- Ship it! Master (a3a35e9) 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 March 21, 2015, 2:22 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/ --- (Updated March 21, 2015, 2:22 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1211 https://issues.apache.org/jira/browse/AURORA-1211 Repository: aurora Description --- Adding a configurable delay into writing a backup file. Diffs - src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java a6b187d888726b921733a36fcaecdab97bdef094 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 569648aea2ccdb57663d16b7c837fd2677694419 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 5853c037d53e707e5df434fc661cd285ed9ecfc4 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java ebe551739fb6b7132ce666ad9b3c5a86e90a5363 Diff: https://reviews.apache.org/r/32359/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 32302: Allow config to specify static ports for non-dedicated jobs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32302/ --- Review request for Aurora. Bugs: AURORA-1212 https://issues.apache.org/jira/browse/AURORA-1212 Repository: aurora Description --- Currently the client allows static ports only for dedicated jobs, the scheduler has no such restirction. Remove this restriction. The docs make no mention of this limition, so no changes are required there. Diffs - src/main/python/apache/aurora/client/config.py 59703ef src/test/python/apache/aurora/client/test_config.py c567797 Diff: https://reviews.apache.org/r/32302/diff/ Testing --- Ran unittests and created a non-dedicated job with a static port in vagrant enviroment. Thanks, Brian Brazil
Re: Review Request 32302: Allow config to specify static ports for non-dedicated jobs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32302/#review77213 --- Ship it! Master (db79418) 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 March 20, 2015, 12:30 p.m., Brian Brazil wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32302/ --- (Updated March 20, 2015, 12:30 p.m.) Review request for Aurora. Bugs: AURORA-1212 https://issues.apache.org/jira/browse/AURORA-1212 Repository: aurora Description --- Currently the client allows static ports only for dedicated jobs, the scheduler has no such restirction. Remove this restriction. The docs make no mention of this limition, so no changes are required there. Diffs - src/main/python/apache/aurora/client/config.py 59703ef src/test/python/apache/aurora/client/test_config.py c567797 Diff: https://reviews.apache.org/r/32302/diff/ Testing --- Ran unittests and created a non-dedicated job with a static port in vagrant enviroment. Thanks, Brian Brazil
Re: Review Request 32231: AURORA-1189: Adding check to see if java version is below 1.8
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32231/#review77222 --- Ship it! Cool, spun this up in vagrant and confirmed: ``` W0320 15:08:49.773 THREAD1 org.apache.aurora.scheduler.app.SchedulerMain.run: ** * * * Beginning with Aurora 0.9.0, you'll need Java 1.8 to run aurora! * Currently you're running 1.7.0_75 * * ** ``` Just one nit, i'll put this on master after the diff is updated. src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java https://reviews.apache.org/r/32231/#comment125098 Remove this, the code below stands well enough on its own. - Bill Farner On March 19, 2015, 9:36 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32231/ --- (Updated March 19, 2015, 9:36 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1189 https://issues.apache.org/jira/browse/AURORA-1189 Repository: aurora Description --- AURORA-1189: Adding check to see if java version is below 1.8 I think it should be fine to check the java version that naive (comparing the char at 3rd posion of java.version) since this check will probably already be removed again until java 1.10 comes out. Diffs - src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 66fb432028c26ea9ed8da475da6c3cbee5e535fc Diff: https://reviews.apache.org/r/32231/diff/ Testing --- * ./gradlew test * running scheduler on vagrant machine to check if log message appears * couldn't get the scheduler run properly on java 1.8 to check if the message doesn't appear there Thanks, Florian Pfeiffer
Review Request 32323: DRY up PMD configuration.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32323/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- DRY up PMD configuration. Diffs - build.gradle 4a13e5c4281de53d0ea991235502943a439d1310 config/pmd/custom.xml 7026d04d8e5c2f185a19c7c44d44698b2fb846c9 config/pmd/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 config/pmd/imports.xml 40dc2260a3c27fd66be352a5974427dcf165ab34 config/pmd/logging-java.xml 66fecbf14aa61f340678dc23d57a1c074cad824a config/pmd/naming.xml 233352d57857a0fe09ddbfe9228b48d4c09bb624 Diff: https://reviews.apache.org/r/32323/diff/ Testing --- ./gradlew -Pq pmdMain -d diffed the Using rule log lines from before and after: ``` % diff -u (sort -u old-rules.txt) (sort -u new-rules.txt) % ``` Thanks, Kevin Sweeney
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77242 --- Realized i neglected to update docs. Please review what's here, new patch will come shortly with doc updates. - Bill Farner On March 20, 2015, 5:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 5:26 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77248 --- Ship it! src/test/python/apache/aurora/client/cli/test_supdate.py https://reviews.apache.org/r/32313/#comment125117 Using a raw mock here is a little bit dangerous if the shape of the raw config changes underneath the test. Would you mind setting the spec argument of the Mock class here? http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock I was thinking of something like Mock(spec=['has_cron_schedule']). Alternatively you could set the spec to be the Job schema object. I think it could be of the form of Mock(spec=aurora.config.schema.base.Job) or Mock(spec=aurora.config.schema.base.Job()). This might be impossible because Job() is a pystachio object but I strongly suggest investigating it. This way if we try to access other attributes the test will fail. - Zameer Manji On March 20, 2015, 10:26 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:26 a.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32231: AURORA-1189: Adding check to see if java version is below 1.8
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32231/ --- (Updated March 20, 2015, 4:36 p.m.) Review request for Aurora and Bill Farner. Changes --- removed the comment Bugs: AURORA-1189 https://issues.apache.org/jira/browse/AURORA-1189 Repository: aurora Description --- AURORA-1189: Adding check to see if java version is below 1.8 I think it should be fine to check the java version that naive (comparing the char at 3rd posion of java.version) since this check will probably already be removed again until java 1.10 comes out. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 66fb432028c26ea9ed8da475da6c3cbee5e535fc Diff: https://reviews.apache.org/r/32231/diff/ Testing --- * ./gradlew test * running scheduler on vagrant machine to check if log message appears * couldn't get the scheduler run properly on java 1.8 to check if the message doesn't appear there Thanks, Florian Pfeiffer
Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77243 --- Ship it! Master (91aec8c) 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 March 20, 2015, 5:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 5:26 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32231: AURORA-1189: Adding check to see if java version is below 1.8
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32231/#review77233 --- Master (3cb8af1) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On March 20, 2015, 4:36 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32231/ --- (Updated March 20, 2015, 4:36 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1189 https://issues.apache.org/jira/browse/AURORA-1189 Repository: aurora Description --- AURORA-1189: Adding check to see if java version is below 1.8 I think it should be fine to check the java version that naive (comparing the char at 3rd posion of java.version) since this check will probably already be removed again until java 1.10 comes out. Diffs - src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 66fb432028c26ea9ed8da475da6c3cbee5e535fc Diff: https://reviews.apache.org/r/32231/diff/ Testing --- * ./gradlew test * running scheduler on vagrant machine to check if log message appears * couldn't get the scheduler run properly on java 1.8 to check if the message doesn't appear there Thanks, Florian Pfeiffer
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77268 --- Ship it! Master (f12d9fe) 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 March 20, 2015, 8:23 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/#review77273 --- Master (f12d9fe) is red with this patch. ./build-support/jenkins/build.sh :startScripts :distTar :distZip :assemble :compileJmhJava :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain :compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java:22: error: cannot find symbol import org.apache.aurora.gen.storage.StoredJob; ^ symbol: class StoredJob location: package org.apache.aurora.gen.storage /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java:22: error: cannot find symbol import org.apache.aurora.gen.storage.StoredJob; ^ symbol: class StoredJob location: package org.apache.aurora.gen.storage /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java:63: error: cannot find symbol assertNull(jobKeyFieldGetterDag.getterFor(StoredJob.class).orNull()); ^ symbol: class StoredJob location: class FieldGetterDagTest 2 errors FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':compileTestJava'. Compilation failed; see the compiler error output for details. * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 1 mins 47.53 secs I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On March 20, 2015, 8:23 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Also the best place to start reviewing FieldGetterDag is also its tests and usage. Diffs - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDag.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java 4e341e05c34b1be38f0040c26b671a0cc797a771 src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5588d1793d6713ee4581ac9f938d9a8689acb315 src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java bdd2185f3a7a94b39bcec3c73455e970d87f0c6a src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java cafd10f6b705568588c1b92644b482003242fe2e src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java ed284f46ac8f01bd6d9e317f995f16d6e666a68d src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java PRE-CREATION
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
On March 20, 2015, 5:59 p.m., Zameer Manji wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, line 101 https://reviews.apache.org/r/32313/diff/1/?file=901501#file901501line101 Using a raw mock here is a little bit dangerous if the shape of the raw config changes underneath the test. Would you mind setting the spec argument of the Mock class here? http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock I was thinking of something like Mock(spec=['has_cron_schedule']). Alternatively you could set the spec to be the Job schema object. I think it could be of the form of Mock(spec=aurora.config.schema.base.Job) or Mock(spec=aurora.config.schema.base.Job()). This might be impossible because Job() is a pystachio object but I strongly suggest investigating it. This way if we try to access other attributes the test will fail. I ventured down this path, but it appears that pystachio is too dynamic for this to be possible [1]. I'll do the spec_set routine, but i fear this will be untenable in other scenarios. [1] https://github.com/wickman/pystachio/blob/master/pystachio/composite.py#L205-L211 - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77248 --- On March 20, 2015, 5:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 5:26 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 7:51 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32323: DRY up PMD configuration.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32323/#review77260 --- Ship it! Master (91aec8c) 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 March 20, 2015, 6:56 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32323/ --- (Updated March 20, 2015, 6:56 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- DRY up PMD configuration. Diffs - build.gradle 4a13e5c4281de53d0ea991235502943a439d1310 config/pmd/custom.xml 7026d04d8e5c2f185a19c7c44d44698b2fb846c9 config/pmd/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 config/pmd/imports.xml 40dc2260a3c27fd66be352a5974427dcf165ab34 config/pmd/logging-java.xml 66fecbf14aa61f340678dc23d57a1c074cad824a config/pmd/naming.xml 233352d57857a0fe09ddbfe9228b48d4c09bb624 Diff: https://reviews.apache.org/r/32323/diff/ Testing --- ./gradlew -Pq pmdMain -d diffed the Using rule log lines from before and after: ``` % diff -u (sort -u old-rules.txt) (sort -u new-rules.txt) % ``` Thanks, Kevin Sweeney
Re: Review Request 32323: DRY up PMD configuration.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32323/#review77261 --- Ship it! Thanks! Good find! - Bill Farner On March 20, 2015, 6:56 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32323/ --- (Updated March 20, 2015, 6:56 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- DRY up PMD configuration. Diffs - build.gradle 4a13e5c4281de53d0ea991235502943a439d1310 config/pmd/custom.xml 7026d04d8e5c2f185a19c7c44d44698b2fb846c9 config/pmd/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 config/pmd/imports.xml 40dc2260a3c27fd66be352a5974427dcf165ab34 config/pmd/logging-java.xml 66fecbf14aa61f340678dc23d57a1c074cad824a config/pmd/naming.xml 233352d57857a0fe09ddbfe9228b48d4c09bb624 Diff: https://reviews.apache.org/r/32323/diff/ Testing --- ./gradlew -Pq pmdMain -d diffed the Using rule log lines from before and after: ``` % diff -u (sort -u old-rules.txt) (sort -u new-rules.txt) % ``` Thanks, Kevin Sweeney
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:15 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Changes --- Kevin rightly pointed out that i could use a real `Job()` instead of a mock, which is better on all counts. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Also the best place to start reviewing FieldGetterDag is also its tests and usage. Diffs - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDag.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java 4e341e05c34b1be38f0040c26b671a0cc797a771 src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5588d1793d6713ee4581ac9f938d9a8689acb315 src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java bdd2185f3a7a94b39bcec3c73455e970d87f0c6a src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java cafd10f6b705568588c1b92644b482003242fe2e src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java ed284f46ac8f01bd6d9e317f995f16d6e666a68d src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java d2ba2730c4509dc9a636fd32e9244b0d7fa2884f src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 1f24e7d47e1f777ffef19a73d01171fcacd31cdb src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b Diff: https://reviews.apache.org/r/32329/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Changes --- Updated doc. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- (Updated March 20, 2015, 3:01 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Changes --- stop referencing nonexistent StoredJob. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Also the best place to start reviewing FieldGetterDag is also its tests and usage. Diffs (updated) - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDag.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java 4e341e05c34b1be38f0040c26b671a0cc797a771 src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5588d1793d6713ee4581ac9f938d9a8689acb315 src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java bdd2185f3a7a94b39bcec3c73455e970d87f0c6a src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java cafd10f6b705568588c1b92644b482003242fe2e src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java ed284f46ac8f01bd6d9e317f995f16d6e666a68d src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java d2ba2730c4509dc9a636fd32e9244b0d7fa2884f src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 1f24e7d47e1f777ffef19a73d01171fcacd31cdb src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b Diff: https://reviews.apache.org/r/32329/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77284 --- Ship it! src/test/python/apache/aurora/client/cli/test_supdate.py https://reviews.apache.org/r/32313/#comment125167 Can we assert `self._mock_api.start_job.mock_calls = [call(...)]` instead? - Joshua Cohen On March 20, 2015, 8:23 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/#review77286 --- Ship it! Master (f12d9fe) 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 March 20, 2015, 10:01 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- (Updated March 20, 2015, 10:01 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Also the best place to start reviewing FieldGetterDag is also its tests and usage. Diffs - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDag.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java 4e341e05c34b1be38f0040c26b671a0cc797a771 src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5588d1793d6713ee4581ac9f938d9a8689acb315 src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java bdd2185f3a7a94b39bcec3c73455e970d87f0c6a src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java cafd10f6b705568588c1b92644b482003242fe2e src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java ed284f46ac8f01bd6d9e317f995f16d6e666a68d src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 src/test/java/org/apache/aurora/scheduler/http/api/security/FieldGetterDagTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java d2ba2730c4509dc9a636fd32e9244b0d7fa2884f src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 1f24e7d47e1f777ffef19a73d01171fcacd31cdb src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b Diff: https://reviews.apache.org/r/32329/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Review Request 32319: Add a deprecation warning when using the client-side updater.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/ --- Review request for Aurora. Bugs: AURORA-1190 https://issues.apache.org/jira/browse/AURORA-1190 Repository: aurora Description --- See summary. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 61337b9312864921508428c7f7992576b94a5d2c src/test/python/apache/aurora/client/cli/test_update.py cc7458568de121dc02de010687fcae0b2707ee0a src/test/python/apache/aurora/client/cli/util.py 864a71428f58ad5ea23beb1d9ae7520c82d2d276 Diff: https://reviews.apache.org/r/32319/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
On March 20, 2015, 10:11 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, lines 158-162 https://reviews.apache.org/r/32313/diff/4/?file=901692#file901692line158 Can we assert `self._mock_api.start_job.mock_calls = [call(...)]` instead? We can! This wasn't done before because validating the resulting `AuroraConfig` was likely deemed some combination of too complex and/or too much coupling. However, you prompted me to discover `mock.ANY`, which fits the bill here. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77284 --- On March 20, 2015, 8:23 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:30 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description (updated) --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:31 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77291 --- Ship it! src/test/python/apache/aurora/client/cli/test_supdate.py https://reviews.apache.org/r/32313/#comment125174 Yes, let us add stuff! Stuff is great and junk! - Joshua Cohen On March 20, 2015, 10:31 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:31 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77292 --- Master (f12d9fe) is red with this patch. ./build-support/jenkins/build.sh SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/common/statuses.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/common/options.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/core/muxer.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/core/inspector.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/core/process.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/core/__init__.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/core/helper.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/core/runner.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/main.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/common.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/help.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/kill.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/run.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/status.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/inspect.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/read.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/simplerun.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/tail.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/commands/gc.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/cli/bin/thermos.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/observed_task.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/task_observer.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/__init__.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/detector.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/templating.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/vars_endpoint.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/file_browser.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/diagnostics.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/configure.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/static_assets.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/__init__.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/http_observer.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/http/json.py Everything Looks Good! SUCCESS: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/observer/bin/__init__.py Everything Looks Good! SUCCESS:
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
On March 20, 2015, 10:33 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, line 636 https://reviews.apache.org/r/32313/diff/5/?file=901848#file901848line636 Yes, let us add stuff! Stuff is great and junk! Stuff=added. This pointed out that i neglected to update these tests to the new style. Key result: mock.patch import is now gone! - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77291 --- On March 20, 2015, 10:31 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:31 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32319: Add a deprecation warning when using the client-side updater.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/#review77295 --- Ship it! Master (f12d9fe) 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 March 20, 2015, 10:27 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/ --- (Updated March 20, 2015, 10:27 p.m.) Review request for Aurora and Joshua Cohen. Bugs: AURORA-1190 https://issues.apache.org/jira/browse/AURORA-1190 Repository: aurora Description --- See summary. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 61337b9312864921508428c7f7992576b94a5d2c src/test/python/apache/aurora/client/cli/test_update.py cc7458568de121dc02de010687fcae0b2707ee0a src/test/python/apache/aurora/client/cli/util.py 864a71428f58ad5ea23beb1d9ae7520c82d2d276 Diff: https://reviews.apache.org/r/32319/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:48 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
