Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72195 --- Ship it! Master (994d669) 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 Feb. 12, 2015, 5:40 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 12, 2015, 5:40 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 12, 2015, 5:40 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Changes --- Stylecheck fixes. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs (updated) - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72068 --- Ship it! FWIW I do agree with Bill's comment that minimum pulse time should be enforced in scheduler rather than here. - David McLaughlin On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72065 --- Ship it! pending clean test run from review bot. - Joshua Cohen On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
On Feb. 11, 2015, 11:59 p.m., David McLaughlin wrote: FWIW I do agree with Bill's comment that minimum pulse time should be enforced in scheduler rather than here. I am +1 on the scheduler check as complementary to the client when/if there is a need for it. I am -1 on using scheduler as the only source of truth here given the reasons I mentioned earlier. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72068 --- On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72091 --- @ReviewBot retry - Maxim Khutornenko On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30461/#review72098
---
Master (61e6c35) 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.collections-0.3.0-py2.7-nspkg.pth
Running setup.py install for twitter.common.util
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
(namespace package)
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
(namespace package)
Installing
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.util-0.3.0-py2.7-nspkg.pth
Running setup.py install for twitter.common.log
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
(namespace package)
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
(namespace package)
Installing
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.log-0.3.0-py2.7-nspkg.pth
Running setup.py install for twitter.common.process
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
(namespace package)
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
(namespace package)
Installing
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.process-0.3.0-py2.7-nspkg.pth
Running setup.py install for gitdb
building 'gitdb._perf' extension
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c
gitdb/_fun.c -o build/temp.linux-x86_64-2.7/gitdb/_fun.o
x86_64-linux-gnu-gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2
-Wall -Wstrict-prototypes -fPIC -Igitdb -I/usr/include/python2.7 -c
gitdb/_delta_apply.c -o build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o
x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions
-Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv
-O2 -Wall -Wstrict-prototypes -D_FORTIFY_SOURCE=2 -g -fstack-protector
--param=ssp-buffer-size=4 -Wformat -Werror=format-security
build/temp.linux-x86_64-2.7/gitdb/_fun.o
build/temp.linux-x86_64-2.7/gitdb/_delta_apply.o -o
build/lib.linux-x86_64-2.7/gitdb/_perf.so
Running setup.py install for twitter.common.app
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py
(namespace package)
Skipping installation of
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py
(namespace package)
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
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review71907 --- Master (64fa0ca) is red with this patch. ./build-support/jenkins/build.sh [1m self.expect_finish()[0m [1m self.replay_mocks()[0m [1m[0m [1m self.update_and_expect_ok()[0m src/test/python/apache/aurora/client/api/test_updater.py:886: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/test/python/apache/aurora/client/api/test_updater.py:338: in update_and_expect_ok [1mself.update_and_expect_response(ResponseCode.OK, instances)[0m _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = aurora.client.api.test_updater.UpdaterTest testMethod=test_update_instances_wait_for_batch_completion_filled_batch expected_code = 1, instances = None message = None [1mdef update_and_expect_response(self, expected_code, instances=None, message=None):[0m [1m resp = self._updater.update(instances)[0m [1m assert expected_code == resp.responseCode, ([0m [1m'Expected response:%s Actual response:%s' % (expected_code, resp.responseCode))[0m [1m[31mE AssertionError: Expected response:1 Actual response:2[0m [1m[31mE assert 1 == 2[0m [1m[31mE + where 2 = Response(details=[ResponseDetail(message='Aborting update without rollback! Fa...n+ UnorderedGroup default')], result=None, responseCode=2, serverInfo=None).responseCode[0m src/test/python/apache/aurora/client/api/test_updater.py:342: AssertionError generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/src.test.python.apache.aurora.client.api.updater.xml [1m[31m== 1 failed, 31 passed in 1.16 seconds ===[0m src.test.python.apache.aurora.client.api.updater . FAILURE src.test.python.apache.aurora.client.cli.config . SUCCESS src.test.python.apache.aurora.client.cli.cron . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS src.test.python.apache.aurora.common.test_http_signaler . SUCCESS src.test.python.apache.aurora.executor.common.path_detector . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.thermos.common.test_pathspec . SUCCESS src.test.python.apache.thermos.monitoring.test_disk . SUCCESS FAILURE [31m FAILURE[0m I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review71900 --- Ship it! Ship It! - Bill Farner On Feb. 11, 2015, 1:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
On Feb. 3, 2015, 12:28 a.m., Bill Farner wrote: src/main/python/apache/aurora/client/api/updater_util.py, line 108 https://reviews.apache.org/r/30461/diff/2/?file=842714#file842714line108 Can you investigate whether the python code respects the `isSetX` pattern? My hunch is that it does, and we should leverage it to distinguish default `0` from the user specifying `0`. This is coming from the schema as `Default(Integer, 0)`. Dropped `Default` completely. On Feb. 3, 2015, 12:28 a.m., Bill Farner wrote: src/main/python/apache/aurora/client/api/updater_util.py, line 25 https://reviews.apache.org/r/30461/diff/2/?file=842714#file842714line25 I suggest increasing this, say to 120. We already have a pretty good safety net built in, i wouldn't want to start battling micro-outages from the beginning. Also, a comment would be useful here to fill in context about what this is, what are the effects of a high/low value. Joshua Cohen wrote: This should probably be renamed to, say, MINUMUM_PULSE_INTERVAL_SECONDS? It doesn't seem to be used as a default anywhere that I see. Given that it's a minimum value, I think 120 seconds is probably too high. Maybe 60? Bill Farner wrote: Oh, if it's a minimum i vote for enforcing it in the scheduler. We don't usually validate ranges for user-defined values on the scheduler. E.g. [1]. I think scheduler should have its own limits enforced when we officially support thrift/REST level job update creations. Until then I'd rather stay consistent with the current approach as having schema variable names (`pulse_interval_secs`) instead of scheduler internal thrift schema names (`blockIfNoPulsesAfterMs`) is more user friendly. [1] - https://github.com/apache/incubator-aurora/blob/3c3b04fd631e6c05e677b298d2c3fd9c5a81ca7d/src/main/python/apache/aurora/client/config.py#L80 Renamed the variable and set the min to 60 seconds. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70662 --- On Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
On Feb. 3, 2015, 3:35 a.m., Joshua Cohen wrote: src/main/python/apache/aurora/client/api/updater.py, line 99 https://reviews.apache.org/r/30461/diff/2/?file=842713#file842713line99 s/in/by the Fixed. On Feb. 3, 2015, 3:35 a.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/api/test_updater_util.py, line 54 https://reviews.apache.org/r/30461/diff/2/?file=842718#file842718line54 instead of hard coding to 10 here, set it to something like `UpdateConfig.DEFAULT_PULSE_INTERVAL_SECS - 1`? Done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70701 --- On Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Feb. 11, 2015, 1:19 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Changes --- CR comments. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs (updated) - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
On Feb. 3, 2015, 12:28 a.m., Bill Farner wrote: src/main/python/apache/aurora/client/api/updater_util.py, line 25 https://reviews.apache.org/r/30461/diff/2/?file=842714#file842714line25 I suggest increasing this, say to 120. We already have a pretty good safety net built in, i wouldn't want to start battling micro-outages from the beginning. Also, a comment would be useful here to fill in context about what this is, what are the effects of a high/low value. Joshua Cohen wrote: This should probably be renamed to, say, MINUMUM_PULSE_INTERVAL_SECONDS? It doesn't seem to be used as a default anywhere that I see. Given that it's a minimum value, I think 120 seconds is probably too high. Maybe 60? Oh, if it's a minimum i vote for enforcing it in the scheduler. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70662 --- On Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70662 --- src/main/python/apache/aurora/client/api/updater_util.py https://reviews.apache.org/r/30461/#comment115985 I suggest increasing this, say to 120. We already have a pretty good safety net built in, i wouldn't want to start battling micro-outages from the beginning. Also, a comment would be useful here to fill in context about what this is, what are the effects of a high/low value. src/main/python/apache/aurora/client/api/updater_util.py https://reviews.apache.org/r/30461/#comment115990 Can you investigate whether the python code respects the `isSetX` pattern? My hunch is that it does, and we should leverage it to distinguish default `0` from the user specifying `0`. src/main/python/apache/aurora/config/schema/base.py https://reviews.apache.org/r/30461/#comment115987 I think we should consider avoiding exposing this to end users. This setting is really a behind-the-scenes timeout that they probably lack the context to set appropriately. - Bill Farner On Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
On Feb. 3, 2015, 12:28 a.m., Bill Farner wrote: src/main/python/apache/aurora/config/schema/base.py, line 35 https://reviews.apache.org/r/30461/diff/2/?file=842715#file842715line35 I think we should consider avoiding exposing this to end users. This setting is really a behind-the-scenes timeout that they probably lack the context to set appropriately. I take that back - this is a policy decision, and OSS end-users should have this knob. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70662 --- On Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
On Feb. 3, 2015, 12:28 a.m., Bill Farner wrote: src/main/python/apache/aurora/client/api/updater_util.py, line 25 https://reviews.apache.org/r/30461/diff/2/?file=842714#file842714line25 I suggest increasing this, say to 120. We already have a pretty good safety net built in, i wouldn't want to start battling micro-outages from the beginning. Also, a comment would be useful here to fill in context about what this is, what are the effects of a high/low value. This should probably be renamed to, say, MINUMUM_PULSE_INTERVAL_SECONDS? It doesn't seem to be used as a default anywhere that I see. Given that it's a minimum value, I think 120 seconds is probably too high. Maybe 60? - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70662 --- On Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70701 --- src/main/python/apache/aurora/client/api/updater.py https://reviews.apache.org/r/30461/#comment116052 s/in/by the src/test/python/apache/aurora/client/api/test_updater_util.py https://reviews.apache.org/r/30461/#comment116054 instead of hard coding to 10 here, set it to something like `UpdateConfig.DEFAULT_PULSE_INTERVAL_SECS - 1`? - Joshua Cohen On Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review70434 --- Ship it! Master (c739122) 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 Jan. 30, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- (Updated Jan. 30, 2015, 10:31 p.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Changes --- Fixing broken test. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs (updated) - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing (updated) --- ./pants test.pytest src/test/python/apache/aurora/client:: Thanks, Maxim Khutornenko
Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30461/#review70427
---
Master (c739122) is red with this patch.
./build-support/jenkins/build.sh
[1mraise AssertionError('Expected call:
%s\nNot called' % (expected,))[0m
[1m[0m
[1mif self.call_args != (args, kwargs):[0m
[1mmsg =
self._format_mock_failure_message(args, kwargs)[0m
[1m raise AssertionError(msg)[0m
[1m[31mE AssertionError: Expected call:
startJobUpdate(JobUpdateRequest(instanceCount=5,
settings=JobUpdateSettings(blockIfNoPulsesAfterMs=None,
updateOnlyTheseInstances=None, maxPerInstanceFailures=2,
maxWaitToInstanceRunningMs=5, waitForBatchCompletion=False,
rollbackOnFailure=True, minWaitInInstanceRunningMs=5, updateGroupSize=1,
maxFailedInstances=1), taskConfig=TaskConfig(isService=None, priority=None,
taskLinks=None, container=Container(docker=None, mesos=MesosContainer()),
executorConfig=None, metadata=None, requestedPorts=None, jobName=None,
environment=None, ramMb=None, job=None, production=None, diskMb=None,
owner=None, maxTaskFailures=None, contactEmail=None, numCpus=None,
constraints=None)))[0m
[1m[31mE Actual call:
startJobUpdate(JobUpdateRequest(instanceCount=5,
settings=JobUpdateSettings(blockIfNoPulsesAfterMs=0,
updateOnlyTheseInstances=None, maxPerInstanceFailures=2,
maxWaitToInstanceRunningMs=5, waitForBatchCompletion=False,
rollbackOnFailure=True, minWaitInInstanceRunningMs=5, updateGroupSize=1,
maxFailedInstances=1), taskConfig=TaskConfig(isService=None, priority=None,
taskLinks=None, container=Container(docker=None, mesos=MesosContainer()),
executorConfig=None, metadata=None, requestedPorts=None, jobName=None,
environment=None, ramMb=None, job=None, production=None, diskMb=None,
owner=None, maxTaskFailures=None, contactEmail=None, numCpus=None,
constraints=None)))[0m
/tmp/user/2395/tmph5gM1a/.deps/mock-1.0.1-py2-none-any.whl/mock.py:835:
AssertionError
generated xml file:
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/src.test.python.apache.aurora.client.api.api.xml
[1m[31m== 1 failed, 10 passed in 0.74 seconds
===[0m
src.test.python.apache.aurora.admin.admin
. SUCCESS
src.test.python.apache.aurora.admin.host_maintenance
. SUCCESS
src.test.python.apache.aurora.admin.maintenance
. SUCCESS
src.test.python.apache.aurora.client.api.api
. FAILURE
src.test.python.apache.aurora.client.api.updater
. SUCCESS
src.test.python.apache.aurora.client.base
. SUCCESS
src.test.python.apache.aurora.client.binding_helper
. SUCCESS
src.test.python.apache.aurora.client.cli.config
. SUCCESS
src.test.python.apache.aurora.client.cli.cron
. SUCCESS
src.test.python.apache.aurora.client.cli.sla
. SUCCESS
src.test.python.apache.aurora.client.config
. SUCCESS
src.test.python.apache.aurora.client.hooks.non_hooked_api
. SUCCESS
src.test.python.apache.aurora.common.test_aurora_job_key
. SUCCESS
src.test.python.apache.aurora.common.test_cluster
. SUCCESS
src.test.python.apache.aurora.common.test_cluster_option
. SUCCESS
src.test.python.apache.aurora.common.test_clusters
. SUCCESS
src.test.python.apache.aurora.common.test_http_signaler
. SUCCESS
src.test.python.apache.aurora.common.test_pex_version
. SUCCESS
src.test.python.apache.aurora.common.test_shellify
. SUCCESS
src.test.python.apache.aurora.common.test_transport
. SUCCESS
Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/ --- Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. Bugs: AURORA-1071 https://issues.apache.org/jira/browse/AURORA-1071 Repository: aurora Description --- - Adding pulse_interval_secs into client UpdateConfig and validating its range - Raising an error in client updater for pulse_interval_secs Diffs - src/main/python/apache/aurora/client/api/updater.py 9f91de625f55514530a4f948d7ecdf7b5614b594 src/main/python/apache/aurora/client/api/updater_util.py 9d2e893a6ecff0fc48c7944575578443d41ced78 src/main/python/apache/aurora/config/schema/base.py e4433d2d47668f59bce169359131284d361bad09 src/test/python/apache/aurora/client/api/test_updater.py dd3f228c5062d388b4393aa4fd5b60a685bdb3a6 src/test/python/apache/aurora/client/api/test_updater_util.py fe3ac49491ca710761632405ac09de0cc0d038a5 Diff: https://reviews.apache.org/r/30461/diff/ Testing --- ./pants test.pytest src/test/python/apache/aurora/client/api:updater ./pants test.pytest src/test/python/apache/aurora/client/api:updater_util Thanks, Maxim Khutornenko
