Re: Review Request 45521: Remove client-side validation of environment names

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45521/#review126276 --- Master (193f17e) is red with this patch.

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-31 Thread Benjamin Staffin
> On March 30, 2016, 11:15 p.m., Aurora ReviewBot wrote: > > Master (193f17e) is red with this patch. > > ./build-support/jenkins/build.sh > > > >  > > self._clock.converge(threads=[hct.threaded_health_checker]) > >  > >

Re: Review Request 45177: Setting DiscoveryInfo.

2016-03-31 Thread Joshua Cohen
> On March 31, 2016, 8:25 p.m., Joshua Cohen wrote: > > RELEASE-NOTES.md, line 14 > > > > > > Is there any reason we need a command line arg to control this? Is > > there any detriment to just always populating

Re: Review Request 45372: Skip flaky test test_health_checker_metrics

2016-03-31 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45372/ --- (Updated March 31, 2016, 3:41 p.m.) Review request for Aurora, John Sirois and

Re: Review Request 45372: Remove sleep and address flaky health check test.

2016-03-31 Thread Bill Farner
> On March 28, 2016, 5:03 a.m., Stephan Erb wrote: > > FWIW, there is als this very old review requests that is talking about the > > same tests https://reviews.apache.org/r/31380/diff/1#index_header. What > > does Brian mean with "calling .converge"? > > John Sirois wrote: > He means the

Re: Review Request 45372: Skip flaky test test_health_checker_metrics

2016-03-31 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45372/#review126485 --- Ship it! Ship It! - John Sirois On March 31, 2016, 4:41

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49 > > > > > > Can we switch on this enum, rather than if/else branching? If we also > >

Re: Review Request 45177: Setting DiscoveryInfo.

2016-03-31 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45177/ --- (Updated March 31, 2016, 9:48 p.m.) Review request for Aurora, Kunal Thakar

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Joshua Cohen
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49 > > > > > > Can we switch on this enum, rather than if/else branching? If we also > >

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Stephan Erb
> On March 31, 2016, 1:17 p.m., Stephan Erb wrote: > > I have no clue about AWS & ELB, so just a couple of notes from the sideline: > > > > * The new feature should be listed in `RELEASE-NOTES.md` > > * We have to figure out how to document that properly (beyond the release > > notes). Right

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/#review126326 --- src/main/python/apache/aurora/common/health_check/shell.py (line

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-31 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45521/#review126344 --- Just to be sure, I saw in the IRC discussion yesterday that Bill

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
> On March 31, 2016, 11:17 a.m., Stephan Erb wrote: > > I have no clue about AWS & ELB, so just a couple of notes from the sideline: > > > > * The new feature should be listed in `RELEASE-NOTES.md` > > * We have to figure out how to document that properly (beyond the release > > notes). Right

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/#review126378 --- The change and the tests LGTM. I currently have great ideas on

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/#review126387 --- Ship it! Master (193f17e) is green with this patch.

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Joshua Cohen
> On March 31, 2016, 6:11 p.m., Zameer Manji wrote: > > The change and the tests LGTM. > > > > I currently have great ideas on how to ensure end to end validation. The > > best idea that I can provide is make use of the shell checker in the e2e > > tests. The program executed by the shell

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Dmitriy Shirchenko
> On March 31, 2016, 4:20 p.m., Dmitriy Shirchenko wrote: > > src/main/python/apache/aurora/common/health_check/shell.py, line 66 > > > > > > why are you doing a separate try/except with timeout when > >

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-31 Thread Bill Farner
> On March 31, 2016, 10 a.m., Joshua Cohen wrote: > > Just to be sure, I saw in the IRC discussion yesterday that Bill suggested > > adding the ability for scheduler-side configuration to replace this. > > > > I'm a strong -1 to this client change without the accompanying scheduler > >

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/ --- (Updated March 31, 2016, 11:01 a.m.) Review request for Aurora, Dmitriy

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Joshua Cohen
> On March 31, 2016, 6:11 p.m., Zameer Manji wrote: > > The change and the tests LGTM. > > > > I currently have great ideas on how to ensure end to end validation. The > > best idea that I can provide is make use of the shell checker in the e2e > > tests. The program executed by the shell

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45511/#review126383 --- src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/ --- (Updated March 31, 2016, 11:57 a.m.) Review request for Aurora, Dmitriy

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
> On March 31, 2016, 11:11 a.m., Zameer Manji wrote: > > The change and the tests LGTM. > > > > I currently have great ideas on how to ensure end to end validation. The > > best idea that I can provide is make use of the shell checker in the e2e > > tests. The program executed by the shell

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/#review126408 --- Ship it! Master (193f17e) is green with this patch.

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
> On March 31, 2016, 6:26 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 54 > > > > > > Is it useful to send a different status code for this state? Seems > >

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
> On March 31, 2016, 9:20 a.m., Dmitriy Shirchenko wrote: > > src/main/python/apache/aurora/common/health_check/shell.py, line 60 > > > > > > why did you get rid of .format? i personally find it much clearer to > >

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Zameer Manji
> On March 31, 2016, 11:11 a.m., Zameer Manji wrote: > > The change and the tests LGTM. > > > > I currently have great ideas on how to ensure end to end validation. The > > best idea that I can provide is make use of the shell checker in the e2e > > tests. The program executed by the shell

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/ --- (Updated March 31, 2016, 11:38 a.m.) Review request for Aurora, Dmitriy

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Dmitriy Shirchenko
> On March 31, 2016, 4:20 p.m., Dmitriy Shirchenko wrote: > > src/main/python/apache/aurora/common/health_check/shell.py, line 60 > > > > > > why did you get rid of .format? i personally find it much clearer to > >

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Dmitriy Shirchenko
> On March 31, 2016, 6:11 p.m., Zameer Manji wrote: > > The change and the tests LGTM. > > > > I currently have great ideas on how to ensure end to end validation. The > > best idea that I can provide is make use of the shell checker in the e2e > > tests. The program executed by the shell

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
> On March 31, 2016, 9:20 a.m., Dmitriy Shirchenko wrote: > > src/main/python/apache/aurora/common/health_check/shell.py, line 66 > > > > > > why are you doing a separate try/except with timeout when > >

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45511/#review126394 --- src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Bill Farner
> On March 31, 2016, 11:26 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 54 > > > > > > Is it useful to send a different status code for this state? Seems > >

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/ --- (Updated March 31, 2016, 1:18 p.m.) Review request for Aurora, Dmitriy

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Bill Farner
> On March 31, 2016, 9:20 a.m., Dmitriy Shirchenko wrote: > > src/main/python/apache/aurora/common/health_check/shell.py, line 66 > > > > > > why are you doing a separate try/except with timeout when > >

Re: Review Request 45177: Setting DiscoveryInfo.

2016-03-31 Thread Joshua Cohen
> On March 31, 2016, 8:25 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line > > 242 > > > > > > Can you also set DiscoveryInfo.location to the cluster name?

Re: Review Request 45177: Setting DiscoveryInfo.

2016-03-31 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45177/ --- (Updated March 31, 2016, 7:51 p.m.) Review request for Aurora. Changes

Re: Review Request 45177: Setting DiscoveryInfo.

2016-03-31 Thread Zhitao Li
> On March 31, 2016, 8:25 p.m., Joshua Cohen wrote: > > RELEASE-NOTES.md, line 14 > > > > > > Is there any reason we need a command line arg to control this? Is > > there any detriment to just always populating

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/#review126443 --- Ship it! Ship It! - Dmitriy Shirchenko On March 31, 2016,

Re: Review Request 45569: add auroraversion to build properties

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45569/#review126506 --- Ship it! Master (17ade11) is green with this patch.

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
> On March 31, 2016, 6:26 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java, lines > > 54-58 > > > > > > `LeaderHealth` only cares about interfacing with

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49 > > > > > > Can we switch on this enum, rather than if/else branching? If we also > >

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
> On March 31, 2016, 6:26 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java, line > > 104 > > > > > > this should be +4 indent, not +8 Done - Ashwin

Re: Review Request 45372: Skip flaky test test_health_checker_metrics

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45372/#review126489 --- Master (17ade11) is red with this patch.

Review Request 45569: add auroraversion to build properties

2016-03-31 Thread Florian Pfeiffer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45569/ --- Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: Aurora-1653

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45511/ --- (Updated April 1, 2016, 4:23 a.m.) Review request for Aurora and Bill Farner.

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45511/#review126535 --- Master (17ade11) is red with this patch.

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45511/#review126545 --- Master (17ade11) is red with this patch.

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45511/#review126297 --- I have no clue about AWS & ELB, so just a couple of notes from

Re: Review Request 45521: Remove client-side validation of environment names

2016-03-31 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45521/#review126298 --- Couple of thoughts on this: * Cool feature! We'd probably use

Re: Review Request 45177: Setting DiscoveryInfo.

2016-03-31 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45177/#review126428 --- RELEASE-NOTES.md (line 14)

Re: Review Request 45506: Execute shell-based health checks as the task user.

2016-03-31 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45506/#review126447 --- Ship it! Ship It! - Zameer Manji On March 31, 2016, 1:18

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-31 Thread Zameer Manji
> On March 29, 2016, 4:20 p.m., John Sirois wrote: > > How would these queries produce incorrect results as compared to previous > > queries? Perviously, a check for the collection being null would have had > > to have been made by the caller (or a check for !IsSet) to skip the calls > >

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-31 Thread Bill Farner
> On March 29, 2016, 4:20 p.m., John Sirois wrote: > > How would these queries produce incorrect results as compared to previous > > queries? Perviously, a check for the collection being null would have had > > to have been made by the caller (or a check for !IsSet) to skip the calls > >

Re: Review Request 45177: Setting DiscoveryInfo.

2016-03-31 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45177/#review126463 --- Master (17ade11) is red with this patch.

Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to detect leading scheduler

2016-03-31 Thread Ashwin Murthy
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49 > > > > > > Can we switch on this enum, rather than if/else branching? If we also > >