Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-16 Thread Stephan Erb
> On Dec. 16, 2016, 2:13 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java, > > lines 69-72 > > > > > > I feel like we should change the default to an empty

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-16 Thread David McLaughlin
> On Dec. 16, 2016, 1:13 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java, > > lines 69-72 > > > > > > I feel like we should change the default to an empty

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-16 Thread Joshua Cohen
> On Dec. 16, 2016, 1:13 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java, > > lines 69-72 > > > > > > I feel like we should change the default to an empty

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-16 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/#review159433 --- Sorry, I am kind of late to the party:

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

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

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/ --- (Updated Dec. 15, 2016, 8:04 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/#review159360 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Dec. 15,

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/#review159359 --- @ReviewBot retry - David McLaughlin On Dec. 15, 2016, 7:24

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/#review159357 --- Master (8e37d0f) is red with this patch.

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/ --- (Updated Dec. 15, 2016, 7:24 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
> On Dec. 15, 2016, 7:21 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java, > > line 128 > > > > > > This `@Timed` annotation is on the wrong method now.

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/ --- (Updated Dec. 15, 2016, 7:22 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/#review159347 --- Fix it, then Ship it! lgtm modulo the below.

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/#review159341 --- Master (8e37d0f) is red with this patch.

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/ --- (Updated Dec. 15, 2016, 6:16 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/#review159336 --- This patch does not apply cleanly against master (8e37d0f), do

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/ --- (Updated Dec. 15, 2016, 6:07 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
> On Dec. 15, 2016, 3:39 p.m., Joshua Cohen wrote: > > I think there's some relevant context in the original review for this > > feature: https://reviews.apache.org/r/44471/. There was concern in that > > review with the removal of the native fields from snapshots for the reasons > > you

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
> On Dec. 15, 2016, 2:15 p.m., Stephan Erb wrote: > > I would like to understand your thinking here: Why add so many todos if the > > necessary code change itself is rather small. Do you expect there is a > > certain risk here? Updated description to match information I added to the JIRA. The

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/ --- (Updated Dec. 15, 2016, 6:01 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54774/ --- (Updated Dec. 15, 2016, 5:59 p.m.) Review request for Aurora, Joshua Cohen,