Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191666 --- Ship it! LGTM. Great new feature! RELEASE-NOTES.md Lines

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread David McLaughlin
> On Nov. 21, 2017, 5:54 p.m., David McLaughlin wrote: > > The outstanding feedback is: > > > > * Discussion on using a mock with capture (my preference) vs using a fake > > implementation. > > * Concern over how to handle a partition during a transient state. My > > preference is to move

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread Bill Farner
> On Nov. 21, 2017, 9:54 a.m., David McLaughlin wrote: > > The outstanding feedback is: > > > > * Discussion on using a mock with capture (my preference) vs using a fake > > implementation. > > * Concern over how to handle a partition during a transient state. My > > preference is to move

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191626 --- Ship it! Master (46b1112) is green with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread Jordan Ly
> On Nov. 21, 2017, 5:54 p.m., David McLaughlin wrote: > > The outstanding feedback is: > > > > * Discussion on using a mock with capture (my preference) vs using a fake > > implementation. > > * Concern over how to handle a partition during a transient state. My > > preference is to move

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 21, 2017, 6:42 p.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191616 --- Ship it! Master (46b1112) is green with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 21, 2017, 6:18 p.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191608 --- Master (46b1112) is red with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread Stephan Erb
> On Nov. 21, 2017, 6:54 p.m., David McLaughlin wrote: > > The outstanding feedback is: > > > > * Discussion on using a mock with capture (my preference) vs using a fake > > implementation. > > * Concern over how to handle a partition during a transient state. My > > preference is to move

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191603 --- The outstanding feedback is: * Discussion on using a mock with

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread David McLaughlin
> On Nov. 16, 2017, 10:01 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java > > Lines 185 (patched) > > > > > > Nit: `isPartitionAware` to minutely

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 21, 2017, 5:50 p.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-16 Thread David McLaughlin
> On Nov. 16, 2017, 10:01 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java > > Lines 209 (patched) > > > > > > I suggest keeping this as a valid transition and

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191261 --- Ship it! Master (46b1112) is green with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-16 Thread David McLaughlin
> On Nov. 16, 2017, 10:22 p.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 90-91 (patched) > > > > > > Can this just be: > > ``` > > if

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-16 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 16, 2017, 10:30 p.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-16 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191256 --- Fix it, then Ship it! Overall LGTM! You may want to add some

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-16 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191229 --- Fix it, then Ship it! Looks great overall! The e2e test is

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191144 --- Ship it! Master (46b1112) is green with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 16, 2017, 1:54 a.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-14 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191025 --- I think this change is sufficiently complex that it warrants an

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191021 --- Ship it! Master (4fecf1f) is green with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-14 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 15, 2017, 12:05 a.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review191013 --- Ship it! Master (4fecf1f) is green with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-14 Thread David McLaughlin
> On Nov. 9, 2017, 5:23 p.m., Bill Farner wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > > Lines 526 (patched) > > > > > > It seems logical that this accounting would live alongside > >

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-14 Thread David McLaughlin
> On Nov. 9, 2017, 1:10 a.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 57 (patched) > > > > > > I don't think you need to `synchronize` here since it is a

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-14 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 14, 2017, 10:41 p.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review190758 --- Master (9b9b2ee) is red with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-10 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 10, 2017, 11:45 p.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread Bill Farner
> On Nov. 9, 2017, 9:23 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 76 (patched) > > > > > > Thinking out loud - the `Optional.of(PARTITIONED)`

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread David McLaughlin
> On Nov. 9, 2017, 5:23 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 76 (patched) > > > > > > Thinking out loud - the `Optional.of(PARTITIONED)`

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread Bill Farner
> On Nov. 9, 2017, 9:23 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 76 (patched) > > > > > > Thinking out loud - the `Optional.of(PARTITIONED)`

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review190632 --- Thanks for all the feedback! I'll move forward towards a

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread David McLaughlin
> On Nov. 9, 2017, 5:23 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 76 (patched) > > > > > > Thinking out loud - the `Optional.of(PARTITIONED)`

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread Jordan Ly
> On Nov. 9, 2017, 1:10 a.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Lines 402 (patched) > > > > > > I wondering if having repeated `PARTITIONED` events would

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review190607 --- Overall approach appears sound. LGTM to proceed to tests.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-09 Thread Bill Farner
> On Nov. 8, 2017, 5:10 p.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Lines 402 (patched) > > > > > > I wondering if having repeated `PARTITIONED` events would

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-08 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review190521 --- Overall the logic seems sound to me!

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review190519 --- Master (773d2d6) is red with this patch.

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-08 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/ --- (Updated Nov. 9, 2017, 12:48 a.m.) Review request for Aurora, Jordan Ly,

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-08 Thread David McLaughlin
> On Nov. 7, 2017, 11:36 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 38-42 (patched) > > > > > > On the other hand, this would be consistent from the

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-07 Thread David McLaughlin
> On Nov. 7, 2017, 11:36 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 38-42 (patched) > > > > > > On the other hand, this would be consistent from the

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-07 Thread David McLaughlin
> On Nov. 7, 2017, 11:36 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/state/PartitionManager.java > > Lines 38-42 (patched) > > > > > > On the other hand, this would be consistent from the

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-07 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review190383 ---

Re: Review Request 63536: Give jobs the ability to determine how to handle partitions by integrating with new Mesos Partition-Aware APIs

2017-11-07 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63536/#review190359 --- Master (ad86177) is red with this patch.