Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Jordan Ly
> On Dec. 14, 2017, 2:07 a.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > > Line 88 (original), 90 (patched) > > > > > > We shouldn't map Edit -> op > >

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Bill Farner
> On Dec. 13, 2017, 6:07 p.m., Jordan Ly wrote: > > Small nits but overall LGTM. In addition to the end-to-end tests passing, > > could you also do a sanity check that this patch works fine on a small > > cluster (upgrade to this patch and downgrade to the previous version, > > ensuring

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64286/#review193756 --- Ship it! Master (4489dc3) is green with this patch.

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Bill Farner
> On Dec. 13, 2017, 6:07 p.m., Jordan Ly wrote: > > Small nits but overall LGTM. In addition to the end-to-end tests passing, > > could you also do a sanity check that this patch works fine on a small > > cluster (upgrade to this patch and downgrade to the previous version, > > ensuring

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64286/#review193751 --- Fix it, then Ship it! Small nits but overall LGTM. In addition

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64286/#review193740 --- Jordan - any remaining comments on the patch? - Bill Farner On

Re: Review Request 64341: Add metadata field to Job object in DSL

2017-12-13 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64341/#review193738 --- Ship it! LGTM! Just one nit with come redundant lines in the