Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114159 --- Ship it! Master (952ef6d) is green with this patch.

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42126/#review114137 --- Ship it! Master (952ef6d) is green with this patch.

Re: Review Request 42177: Vagrant change to reserve part of the dev cluster's resources to 'aurora-role'

2016-01-12 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42177/ --- (Updated Jan. 12, 2016, 8:04 p.m.) Review request for Aurora, Maxim

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
> On Jan. 13, 2016, 1:31 a.m., Bill Farner wrote: > > It sounds like you ran into an issue that necessitated this patch. Is > > there anything you can add to `docs/security.md` so others don't get stuck > > down the same dead end? > > > > Also, please add a NEWS entry summarizing the new

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/ --- (Updated Jan. 13, 2016, 7:20 a.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42126/#review114128 --- Thanks for the changes so far, nearly there!

Re: Review Request 42126: New interface to allocate resources of multiple roles from offer.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42126/#review114000 --- src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/ --- (Updated Jan. 12, 2016, 6:52 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
> On Jan. 12, 2016, 5:41 p.m., Joshua Cohen wrote: > > src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java, > > line 126 > > > > > > Do we need to use a `StatsProvider` for this? Can we

Re: Review Request 42077: Introduces -default_docker_parameters scheduler flag.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42077/#review114038 --- I'm good for a ship it once a NEWS entry is added.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
> On Jan. 12, 2016, 5:41 p.m., Joshua Cohen wrote: > > It would've been nice if, by default, we did not wire up the post-filter at > > all, and only asserted its counters when it has been wired in, but from a > > quick glance at the way the tests are set up, it doesn't seem to be > > possible

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114054 --- Ship it! Master (d542bd1) is green with this patch.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114060 --- Ship it! Ship It! - Joshua Cohen On Jan. 12, 2016, 6:52 p.m.,

Re: Review Request 42077: Introduces -default_docker_parameters scheduler flag.

2016-01-12 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42077/#review114058 --- Ship it! Ship It! - Stephan Erb On Jan. 12, 2016, 8:58 p.m.,

Re: Review Request 42077: Introduces -default_docker_parameters scheduler flag.

2016-01-12 Thread George Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42077/ --- (Updated Jan. 12, 2016, 7:58 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 42077: Introduces -default_docker_parameters scheduler flag.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42077/#review114056 --- Ship it! Ship It! - Bill Farner On Jan. 12, 2016, 11:58 a.m.,

Re: Review Request 42077: Introduces -default_docker_parameters scheduler flag.

2016-01-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42077/#review114064 --- Ship it! Master (d542bd1) is green with this patch.

Re: Review Request 42126: New interface to allocate resources of multiple roles from offer.

2016-01-12 Thread Bill Farner
> On Jan. 11, 2016, 3:42 p.m., Zhitao Li wrote: > > src/main/java/org/apache/aurora/scheduler/OfferAllocation.java, line 188 > > > > > > Hmm, I think I'd like to keep this for two reasons: > > > > 1. Even

Re: Review Request 42225: Add `--show-error` to curl when bootstrapping thrift.

2016-01-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42225/#review114112 --- Ship it! Master (d542bd1) is green with this patch.

Review Request 42225: Add `--show-error` to curl when bootstrapping thrift.

2016-01-12 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42225/ --- Review request for Aurora and John Sirois. Repository: aurora Description

Re: Review Request 42225: Add `--show-error` to curl when bootstrapping thrift.

2016-01-12 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42225/#review114108 --- Ship it! I almost always use -sS for this very reason - this

Re: Review Request 42225: Add `--show-error` to curl when bootstrapping thrift.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42225/#review114109 --- Ship it! Ship It! - Bill Farner On Jan. 12, 2016, 3:51 p.m.,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114103 --- It sounds like you ran into an issue that necessitated this patch.

Re: Review Request 42126: New interface to allocate resources of multiple roles from offer.

2016-01-12 Thread Zhitao Li
> On Jan. 12, 2016, 6:05 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/AcceptedOffer.java, line 40 > > > > > > The interface should really be removed. It's not acting as an > > interface

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42126/ --- (Updated Jan. 13, 2016, 1:34 a.m.) Review request for Aurora, Maxim

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42126/#review114127 --- src/main/java/org/apache/aurora/scheduler/Resources.java (line

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread John Sirois
> On Jan. 12, 2016, 6:49 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/Resources.java, line 50 > > > > > > Looks like an overly-aggressive find/replace. I've been endlessly > > frustrated

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42126/#review114130 --- Ship it! Master (952ef6d) is green with this patch.

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread John Sirois
> On Jan. 12, 2016, 6:49 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/Resources.java, line 50 > > > > > > Looks like an overly-aggressive find/replace. I've been endlessly > > frustrated

Re: Review Request 42126: New class to allocate resources of multiple roles from offer.

2016-01-12 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42126/ --- (Updated Jan. 13, 2016, 2:08 a.m.) Review request for Aurora, Maxim

Re: Review Request 40877: Update rpm startup scripts to match deb patterns in use

2016-01-12 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40877/#review114005 --- Jake - I'm willing to take this patch over if you don't have time

Re: Review Request 42126: New interface to allocate resources of multiple roles from offer.

2016-01-12 Thread Bill Farner
> On Jan. 11, 2016, 8:25 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line > > 201 > > > > > > Is `clearResources()` necessary here? I think it makes sense to

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114009 --- It would've been nice if, by default, we did not wire up the