Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/#review118665 --- Ship it! This lgtm, all small stuff.

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread John Sirois
> On Feb. 10, 2016, 8:57 a.m., John Sirois wrote: > > src/main/python/apache/aurora/client/cli/jobs.py, line 114 > > > > > > I'm not sure what the current guiding philosophy in the client is, but > > I'd be more

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread John Sirois
> On Feb. 10, 2016, 8:57 a.m., John Sirois wrote: > > src/main/python/apache/aurora/client/cli/jobs.py, line 114 > > > > > > I'm not sure what the current guiding philosophy in the client is, but > > I'd be more

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/ --- (Updated Feb. 10, 2016, 6:20 p.m.) Review request for Aurora, John Sirois and

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread John Sirois
> On Feb. 10, 2016, 8:57 a.m., John Sirois wrote: > > src/main/python/apache/aurora/client/cli/context.py, line 195 > > > > > > This is a lie as is the method name, this returns a set of int and not > > a list of

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/#review118706 --- Ship it! Master (86b6d79) is green with this patch.

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Maxim Khutornenko
> On Feb. 10, 2016, 3:57 p.m., John Sirois wrote: > > src/main/python/apache/aurora/client/cli/jobs.py, line 114 > > > > > > I'm not sure what the current guiding philosophy in the client is, but > > I'd be more

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/#review118699 --- Ship it! Ship It! - John Sirois On Feb. 10, 2016, 11:33

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/ --- (Updated Feb. 10, 2016, 6:33 p.m.) Review request for Aurora, John Sirois and

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/#review118703 --- Sorry to back out late - i don't have capacity for a decent

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Maxim Khutornenko
> On Feb. 10, 2016, 9:39 p.m., Stephan Erb wrote: > > Stupid question: Will there be another review request implementing instance > > removal? It already exists: `aurora job kill` - Maxim --- This is an automatically generated e-mail.

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/#review118728 --- Stupid question: Will there be another review request

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/#review118707 --- Ship it! Ship It! - Joshua Cohen On Feb. 10, 2016, 6:33

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/#review118784 --- @ReviewBot retry - Zameer Manji On Feb. 10, 2016, 4:35 p.m.,

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread John Sirois
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/#review118772 --- > and removes the population of an object via a constructor which

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/#review118775 --- Please substitute Maxim for me on this review. - Bill Farner

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/#review118783 --- Master (46277a1) is red with this patch.

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/#review118774 ---

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Maxim Khutornenko
> On Feb. 11, 2016, 12:27 a.m., Bill Farner wrote: > > Please substitute Maxim for me on this review. I am already in the list :) - Maxim --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Zameer Manji
> On Feb. 10, 2016, 4:27 p.m., Bill Farner wrote: > > Please substitute Maxim for me on this review. > > Maxim Khutornenko wrote: > I am already in the list :) Maixm is already on this review. I have replaced you with John. - Zameer

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread John Sirois
> On Feb. 10, 2016, 5:33 p.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml, > > line 83 > > > > > > You may improve perf even further if you include `id`

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Zameer Manji
> On Feb. 10, 2016, 4:23 p.m., John Sirois wrote: > > > and removes the population of an object via a constructor which is slower > > > than populating an object via setters. > > > > For clarification, is this optimization the much smaller of the 2? Naively > > - I'd hope construction would

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Zameer Manji
> On Feb. 10, 2016, 4:57 p.m., Bill Farner wrote: > > It would be nice to hear how this change jives with the opposite change > > made in https://reviews.apache.org/r/42882 > > Maxim Khutornenko wrote: > I thought about that too. I think there are 2 major differences: the > total number

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Stephan Erb
> On Feb. 10, 2016, 10:39 p.m., Stephan Erb wrote: > > Stupid question: Will there be another review request implementing instance > > removal? > > Maxim Khutornenko wrote: > It already exists: `aurora job kill` I feel like this exposes an inconsistency of the API: * `add` appends some

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Stephan Erb
> On Feb. 10, 2016, 10:39 p.m., Stephan Erb wrote: > > Stupid question: Will there be another review request implementing instance > > removal? > > Maxim Khutornenko wrote: > It already exists: `aurora job kill` > > Stephan Erb wrote: > I feel like this exposes an inconsistency of the

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Maxim Khutornenko
> On Feb. 10, 2016, 11:16 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/client/cli/jobs.py, line 474 > > > > > > This might be subject to race conditions, i.e. if anyone else is > > messing with the

Review Request 43458: Documenting aurora job add command.

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

Re: Review Request 43458: Documenting aurora job add command.

2016-02-10 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43458/#review118771 --- Ship it! Ship It! - Stephan Erb On Feb. 11, 2016, 1 a.m.,

Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository:

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43373/#review118754 --- src/main/python/apache/aurora/client/cli/jobs.py (line 468)

Re: Review Request 43373: Implementing 'aurora job add' command.

2016-02-10 Thread Maxim Khutornenko
> On Feb. 10, 2016, 9:39 p.m., Stephan Erb wrote: > > Stupid question: Will there be another review request implementing instance > > removal? > > Maxim Khutornenko wrote: > It already exists: `aurora job kill` > > Stephan Erb wrote: > I feel like this exposes an inconsistency of the

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Zameer Manji
> On Feb. 10, 2016, 4:23 p.m., John Sirois wrote: > > > and removes the population of an object via a constructor which is slower > > > than populating an object via setters. > > > > For clarification, is this optimization the much smaller of the 2? Naively > > - I'd hope construction would

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/#review118790 --- It would be nice to hear how this change jives with the opposite

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread John Sirois
> On Feb. 10, 2016, 5:23 p.m., John Sirois wrote: > > > and removes the population of an object via a constructor which is slower > > > than populating an object via setters. > > > > For clarification, is this optimization the much smaller of the 2? Naively > > - I'd hope construction would

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread John Sirois
> On Feb. 10, 2016, 5:33 p.m., Maxim Khutornenko wrote: > > src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml, > > line 83 > > > > > > You may improve perf even further if you include `id`

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43457/#review118800 --- Master (e7862e0) is green with this patch.

Re: Review Request 43457: Increase throughput of DbTaskStore

2016-02-10 Thread Maxim Khutornenko
> On Feb. 11, 2016, 12:57 a.m., Bill Farner wrote: > > It would be nice to hear how this change jives with the opposite change > > made in https://reviews.apache.org/r/42882 I thought about that too. I think there are 2 major differences: the total number of rows generated by the multi-join

Re: Review Request 43458: Documenting aurora job add command.

2016-02-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43458/#review118795 --- Ship it! Master (46277a1) is green with this patch.