Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117902 --- Cherry-picked reverted patches and ran upgrade/rollback cycle twic

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117886 --- Ship it! Master (42bff19) is green with this patch. ./build-s

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/ --- (Updated Feb. 4, 2016, 8:58 p.m.) Review request for Aurora, John Sirois and Bi

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117875 --- Ship it! Master (42bff19) is green with this patch. ./build-s

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117839 --- Ship it! Ship It! - John Sirois On Feb. 4, 2016, 10:13 a.m.,

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/ --- (Updated Feb. 4, 2016, 5:13 p.m.) Review request for Aurora, John Sirois and Bi

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 10:37 a.m., Stephan Erb wrote: > > During this whole odyssee, have we learned a thing which is not reflected > > here? > > https://github.com/apache/aurora/blob/master/docs/thrift-deprecation.md > > > > If yes, please write it down :-) > > John Sirois wrote: > I concur,

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 4:36 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java, > > line 47 > > > > > > Please favor immutable inputs and outputs; return `IJobUpdate`.

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 2:21 p.m., John Sirois wrote: > > src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java, > > line 50 > > > > > > Finish this thought - there is no actual testing done here.

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 6:21 a.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, > > line 45 > > > > > > Having a backfill in the db layer strikes me as a red flag; ie

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 2:21 p.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, > > line 45 > > > > > > Having a backfill in the db layer strikes me as a red flag; ie

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117831 --- Ship it! src/main/java/org/apache/aurora/scheduler/storage/log

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 6:21 a.m., John Sirois wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, > > line 45 > > > > > > Having a backfill in the db layer strikes me as a red flag; ie

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117808 --- Fix it, then Ship it! src/main/java/org/apache/aurora/schedule

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 3:37 a.m., Stephan Erb wrote: > > During this whole odyssee, have we learned a thing which is not reflected > > here? > > https://github.com/apache/aurora/blob/master/docs/thrift-deprecation.md > > > > If yes, please write it down :-) I concur, but I'm fine with the doc upd

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117791 --- During this whole odyssee, have we learned a thing which is not re

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 4, 2016, 2:39 a.m., John Sirois wrote: > > This looks about right to me. > > > > In the general case though for thrift field moves (I was thinking about > > this problem last night), it seems to me there would need to be a > > bi-directional fill in-place when the new field locations

Re: Review Request 43172: Add deprecated field storage backfill

> On Feb. 3, 2016, 7:39 p.m., John Sirois wrote: > > This looks about right to me. > > > > In the general case though for thrift field moves (I was thinking about > > this problem last night), it seems to me there would need to be a > > bi-directional fill in-place when the new field locations

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117750 --- This looks about right to me. In the general case though for thri

Re: Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/#review117751 --- Ship it! Master (52c19cf) is green with this patch. ./build-s

Review Request 43172: Add deprecated field storage backfill

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43172/ --- Review request for Aurora, John Sirois and Bill Farner. Bugs: AURORA-1603 h