Re: Review Request 24206: Add an UpdateStrategy abstraction, with two implementations.

2014-08-04 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24206/#review49466 --- Ship it! Ship It!

Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/ --- Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.

Review Request 24241: Add test coverage for new client API methods.

2014-08-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24241/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora

Re: Review Request 24241: Add test coverage for new client API methods.

2014-08-04 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24241/#review49475 --- Ship it! Thanks! - Maxim Khutornenko On Aug. 4, 2014, 6:29

Re: Review Request 24160: Add deprecation warnings for aurora client v1 command.

2014-08-04 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24160/#review49476 --- Ship it! At first I was wondering how we're going to remove these

Re: Review Request 24160: Add deprecation warnings for aurora client v1 command.

2014-08-04 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24160/#review49477 --- Ship it! At first I was wondering how we're going to remove these

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/#review49487 ---

Re: Review Request 24206: Add an UpdateStrategy abstraction, with two implementations.

2014-08-04 Thread Bill Farner
On Aug. 4, 2014, 5:34 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java, line 36 https://reviews.apache.org/r/24206/diff/1/?file=649183#file649183line36 protected? Done. On Aug. 4, 2014, 5:34 p.m., Kevin Sweeney

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/ --- (Updated Aug. 4, 2014, 8:28 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/#review49492 --- partial review on v1, will deliver full with v2

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Maxim Khutornenko
On Aug. 4, 2014, 8:31 p.m., Kevin Sweeney wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, lines 101-102 https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line101 Is there a reason not to use TIMESTAMP [1] types that will be

Re: Review Request 23863: AURORA-587: Example ServerSet Announcer implementation

2014-08-04 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23863/#review49501 --- sorry for the radio silence on this -- vacation + other work. will

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Bill Farner
On Aug. 4, 2014, 8:31 p.m., Kevin Sweeney wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, lines 101-102 https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line101 Is there a reason not to use TIMESTAMP [1] types that will be

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Kevin Sweeney
On Aug. 4, 2014, 1:31 p.m., Kevin Sweeney wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 111 https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line111 Will the interface return RangeSetInteger? Maybe a more compact form could

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/#review49505 --- Ship it! +1 from me after the double-negative is removed - Kevin

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/ --- (Updated Aug. 4, 2014, 8:59 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Bill Farner
On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 123 https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line123 I'm an anti-fan of flags that say do not do X, as they can lead to confusing

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/ --- (Updated Aug. 4, 2014, 9:04 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 23863: AURORA-587: Example ServerSet Announcer implementation

2014-08-04 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23863/ --- (Updated Aug. 4, 2014, 9:14 p.m.) Review request for Aurora, Kevin Sweeney and

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/#review49511 --- FlagSchemaChanges is going to fail if this is committed as-is. -

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/#review49510 --- Ship it!

Re: Review Request 23863: AURORA-587: Example ServerSet Announcer implementation

2014-08-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23863/#review49517 --- Neglected to catch this in previous rounds, but can you add a

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread Maxim Khutornenko
On Aug. 4, 2014, 9:41 p.m., Bill Farner wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 98 https://reviews.apache.org/r/24243/diff/4/?file=650982#file650982line98 Strongly consider dropping update_id to let the IDENTITY stand alone. If we were

Re: Review Request 24243: DB schema for the job update store.

2014-08-04 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24243/#review49523 --- Ship it! Ship It! - David McLaughlin On Aug. 4, 2014, 10:06

Re: Review Request 24261: Add deprecation warnings to other clientv1 commands.

2014-08-04 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24261/#review49525 --- Ship it! Ship It! - Joe Smith On Aug. 4, 2014, 1:59 p.m., Mark

Re: Review Request 23863: AURORA-587: Example ServerSet Announcer implementation

2014-08-04 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23863/ --- (Updated Aug. 4, 2014, 11:01 p.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 23863: AURORA-587: Example ServerSet Announcer implementation

2014-08-04 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23863/ --- (Updated Aug. 4, 2014, 11:02 p.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 23863: AURORA-587: Example ServerSet Announcer implementation

2014-08-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23863/#review49526 --- Ship it! Ship It! - Bill Farner On Aug. 4, 2014, 11:02 p.m.,

Re: Review Request 23863: AURORA-587: Example ServerSet Announcer implementation

2014-08-04 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23863/#review49527 --- Ship it! Ship It! - Kevin Sweeney On Aug. 4, 2014, 4:02 p.m.,

Re: Review Request 24261: Add deprecation warnings to other clientv1 commands.

2014-08-04 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24261/#review49529 --- Ship it! Ship It! - David McLaughlin On Aug. 4, 2014, 8:59

Review Request 24281: Initial implementation of the UpdateStore (saveUpdate).

2014-08-04 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24281/ --- Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.

Re: Review Request 24281: Initial implementation of the UpdateStore (saveUpdate).

2014-08-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24281/#review49530 ---

Re: Review Request 24281: Initial implementation of the UpdateStore (saveUpdate).

2014-08-04 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24281/#review49533 --- Ship it! Ship It! - Kevin Sweeney On Aug. 4, 2014, 4:30 p.m.,

Re: Review Request 24281: Initial implementation of the UpdateStore (saveUpdate).

2014-08-04 Thread Maxim Khutornenko
On Aug. 4, 2014, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java, line 44 https://reviews.apache.org/r/24281/diff/1/?file=651305#file651305line44 Does this compile? Maxim Khutornenko wrote: Everything build and checks

Re: Review Request 24281: Initial implementation of the UpdateStore (saveUpdate).

2014-08-04 Thread Bill Farner
On Aug. 4, 2014, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java, line 44 https://reviews.apache.org/r/24281/diff/1/?file=651305#file651305line44 Does this compile? Maxim Khutornenko wrote: Everything build and checks

Re: Review Request 24281: Initial implementation of the UpdateStore (saveUpdate).

2014-08-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24281/#review49537 --- Ship it! LGTM mod naming

Re: Review Request 24281: Initial implementation of the UpdateStore (saveUpdate).

2014-08-04 Thread Maxim Khutornenko
On Aug. 4, 2014, 11:53 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/UpdateStore.java, line 21 https://reviews.apache.org/r/24281/diff/1/?file=651307#file651307line21 Before this continues much futher, i'd love to see s/Update/JobUpdate/ to