Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Zameer Manji
> On Aug. 30, 2016, 12:37 a.m., Stephan Erb wrote: > > Were you able to test this on a cluster with concurrent modifications? > > Right now, I am not confident this is working as most ouf our thrift > > responses contain unordered sequences. I would therefore suspect that we > > get a

Re: Review Request 51536: @ReviewBot retry Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Kai Huang
> On Aug. 30, 2016, 9:25 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java, > > line 143 > > > > > > We should add a constant to api.thrift for `health` and use it

Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Zameer Manji
> On Aug. 30, 2016, 12:37 a.m., Stephan Erb wrote: > > Were you able to test this on a cluster with concurrent modifications? > > Right now, I am not confident this is working as most ouf our thrift > > responses contain unordered sequences. I would therefore suspect that we > > get a

Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Zameer Manji
> On Aug. 29, 2016, 6:32 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java, > > lines 139-142 > > > > > > Am I reading this correctly: you're buffering the

Re: Review Request 51536: @ReviewBot retry Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Kai Huang
> On Aug. 30, 2016, 9:25 p.m., Joshua Cohen wrote: > > src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java, > > lines 191-201 > > > > > > Beyond setting things up to use the new style

Re: Review Request 51536: @ReviewBot retry Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Stephan Erb
> On Aug. 30, 2016, 11:25 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java, > > line 143 > > > > > > We should add a constant to api.thrift for `health` and use

Re: Review Request 51536: @ReviewBot retry Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51536/#review147358 ---

Re: Review Request 51536: @ReviewBot retry Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51536/#review147359 --- Ship it! Master (c99f2fb) is green with this patch.

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois
> On Aug. 30, 2016, 11:56 a.m., Zameer Manji wrote: > > LGTM modulo updating the docs about restarting all instances at the same > > time. > > > > Could you also file a ticket to track the removal of the `zk_use_curator` > > flag in 0.17? I did not see one on JIRA. > > John Sirois wrote: >

Re: Review Request 51536: @ReviewBot retry Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51536/ --- (Updated Aug. 30, 2016, 8:52 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/#review147355 --- Master (c99f2fb) is green with this patch.

Re: Review Request 51536: @ReviewBot retry Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51536/ --- (Updated Aug. 30, 2016, 8:37 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Zameer Manji
> On Aug. 30, 2016, 10:56 a.m., Zameer Manji wrote: > > LGTM modulo updating the docs about restarting all instances at the same > > time. > > > > Could you also file a ticket to track the removal of the `zk_use_curator` > > flag in 0.17? I did not see one on JIRA. > > John Sirois wrote: >

Re: Review Request 51499: Update 3dparty Python dependencies

2016-08-30 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51499/#review147353 --- Ship it! Overall LGTM. A few suggestions/comments: * In your

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/ --- (Updated Aug. 30, 2016, 2:25 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/#review147347 --- This patch does not apply cleanly against master (c99f2fb), do

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/#review147348 --- Ship it! Ship It! - Joshua Cohen On Aug. 30, 2016, 8 p.m.,

Re: Review Request 51535: Fix a Python unittest that is not asserting anything

2016-08-30 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51535/#review147346 --- Ship it! Good catch! - Maxim Khutornenko On Aug. 30, 2016,

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/ --- (Updated Aug. 30, 2016, 2 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois
> On Aug. 30, 2016, 12:23 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java, > > line 78 > > > > > > I suggest against placing a version number into

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois
> On Aug. 30, 2016, 1:23 a.m., Stephan Erb wrote: > > RELEASE-NOTES.md, lines 41-42 > > > > > > You once recommended to stop all schedules at once when switching to > > Curator, rather than doing this in a rolling

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread John Sirois
> On Aug. 30, 2016, 11:56 a.m., Zameer Manji wrote: > > LGTM modulo updating the docs about restarting all instances at the same > > time. > > > > Could you also file a ticket to track the removal of the `zk_use_curator` > > flag in 0.17? I did not see one on JIRA. The language of

Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Stephan Erb
> On Aug. 30, 2016, 9:37 a.m., Stephan Erb wrote: > > Were you able to test this on a cluster with concurrent modifications? > > Right now, I am not confident this is working as most ouf our thrift > > responses contain unordered sequences. I would therefore suspect that we > > get a

Review Request 51536: Scheduler updater will not use watch_sec if health check is enabled

2016-08-30 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51536/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Repository:

Review Request 51535: Fix a Python unittest that is not asserting anything

2016-08-30 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51535/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora

Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Joshua Cohen
> On Aug. 30, 2016, 1:32 a.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java, > > lines 139-142 > > > > > > Am I reading this correctly: you're buffering the

Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Zameer Manji
> On Aug. 30, 2016, 12:37 a.m., Stephan Erb wrote: > > Were you able to test this on a cluster with concurrent modifications? > > Right now, I am not confident this is working as most ouf our thrift > > responses contain unordered sequences. I would therefore suspect that we > > get a

Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Zameer Manji
> On Aug. 29, 2016, 6:32 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java, > > lines 139-142 > > > > > > Am I reading this correctly: you're buffering the

Re: Review Request 51531: Minor improvements to the custom executor docs

2016-08-30 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51531/#review147329 --- Ship it! Ship It! - Joshua Cohen On Aug. 30, 2016, 5:45

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Joshua Cohen
> On Aug. 30, 2016, 6:23 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java, > > line 78 > > > > > > I suggest against placing a version number into

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/#review147326 ---

Re: Review Request 51531: Minor improvements to the custom executor docs

2016-08-30 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51531/#review147324 --- Ship it! Ship It! - Zameer Manji On Aug. 30, 2016, 10:45

Re: Review Request 51531: Minor improvements to the custom executor docs

2016-08-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51531/#review147323 --- Ship it! Master (c34f78a) is green with this patch.

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/#review147322 --- Ship it! LGTM modulo updating the docs about restarting all

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Zameer Manji
> On Aug. 30, 2016, 12:23 a.m., Stephan Erb wrote: > > RELEASE-NOTES.md, lines 41-42 > > > > > > You once recommended to stop all schedules at once when switching to > > Curator, rather than doing this in a rolling

Review Request 51531: Minor improvements to the custom executor docs

2016-08-30 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51531/ --- Review request for Aurora and Joshua Cohen. Repository: aurora Description

Re: Review Request 51384: Introduce UpdateMetadata fields in JobUpdateRequest.

2016-08-30 Thread Joshua Cohen
> On Aug. 29, 2016, 6:52 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/client/cli/update.py, line 193 > > > > > > Am I missing something? Shouldn't this be a set of `Metadata` structs? > > rather than

Re: Review Request 51499: Update 3dparty Python dependencies

2016-08-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51499/#review147274 --- Ship it! Master (c34f78a) is green with this patch.

Re: Review Request 51499: Update 3dparty Python dependencies

2016-08-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51499/#review147272 --- Master (c34f78a) is red with this patch.

Re: Review Request 51384: Introduce UpdateMetadata fields in JobUpdateRequest.

2016-08-30 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51384/#review147271 --- Please don't forget to add an entry to the release notes :-) -

Re: Review Request 51499: Update 3dparty Python dependencies

2016-08-30 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51499/ --- (Updated Aug. 30, 2016, 9:40 a.m.) Review request for Aurora, John Sirois and

Re: Review Request 51513: Add support for ETags in the Aurora API.

2016-08-30 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51513/#review147270 --- Were you able to test this on a cluster with concurrent

Re: Review Request 51506: Enable -zk_use_curator by default and deprecate.

2016-08-30 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51506/#review147269 --- Fix it, then Ship it! RELEASE-NOTES.md (lines 41 - 42)