Re: Review Request 64234: Extract a storage Persistence layer

2017-11-30 Thread Bill Farner
IT.java > a1944c4fdaa5bc578a7c1430f6304eae5d8a5f08 > > src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java > 59c2c5bb1a4bea125ae799d76179946b722fc150 > > src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java > 8a99b36757a25f58773c5445c046c4f9f9158056 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 42a79a6ca6ee0f51b7d1a4ac1df9dbd555f2d2b9 > src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java > b2c371cde5eec186a1bbfbfc8a0555f9930a6791 > > > Diff: https://reviews.apache.org/r/64234/diff/1/ > > > Testing > --- > > end-to-end tests pass > > > Thanks, > > Bill Farner > >

Re: Review Request 64234: Extract a storage Persistence layer

2017-11-30 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64234/#review192444 --- @ReviewBot retry - Bill Farner On Nov. 30, 2017, 4:32 p.m

Re: Review Request 60942: Remove task level resource fields from thrift interface and db

2017-11-27 Thread Bill Farner
> On Nov. 23, 2017, 12:46 p.m., Stephan Erb wrote: > > RELEASE-NOTES.md > > Lines 57 (patched) > > > > > > You will need to move this to 0.20.0. Also please state that you > > removed it. The fields have been

Re: Review Request 60942: Remove task level resource fields from thrift interface and db

2017-11-27 Thread Bill Farner
u > > removed it. The fields have been deprecated before. > > Bill Farner wrote: > Note - this line is still in the 0.19.0 section in the latest diff, and > needs to be moved up to 0.20.0 as Stephan points out. > > Nicolás Donatucci wrote: > In latest diff, revision

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Bill Farner
ws.apache.org/r/64288/#review192666 --- On Dec. 3, 2017, 5:09 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > h

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Bill Farner
review192722 --- On Dec. 3, 2017, 5:09 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Bill Farner
, Bill Farner

Review Request 64287: Clean up some lint rules

2017-12-03 Thread Bill Farner
9e956deb20bd4ec4821f4793d1a1cd18ed361c53 Diff: https://reviews.apache.org/r/64287/diff/1/ Testing --- Thanks, Bill Farner

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

2017-12-17 Thread Bill Farner
. - Bill Farner On Dec. 17, 2017, 7:24 a.m., Jing Chen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-12 Thread Bill Farner
ty reasonable) I think that's the right order of magnitude. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review193234 -------

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-12 Thread Bill Farner
happy if the build is happy! - Bill Farner On Dec. 12, 2017, 11:15 a.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

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

2017-12-12 Thread Bill Farner
.lang.Thread.run(Thread.java:748) E1212 17:14:46.570 [SnapshotService$$EnhancerByGuice$$9e5ef4c0 RUNNING, GuavaUtils$LifecycleShutdownListener] Service: SnapshotService$$EnhancerByGuice$$9e5ef4c0 [FAILED] failed unexpectedly. Tr iggering shutdown. ``` - Bill --------

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

2017-12-12 Thread Bill Farner
/64286/diff/3/ Changes: https://reviews.apache.org/r/64286/diff/2-3/ Testing --- end-to-end tests pass Thanks, Bill Farner

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-12 Thread Bill Farner
> On Dec. 11, 2017, 6:57 p.m., Bill Farner wrote: > > I should have looked more closely at the last patch, and when preparing my > > patch before that. This test is bound to be flaky since there's nothing > > ensuring a happens-before releationship betwee

Review Request 64625: Add a storage recovery tool

2017-12-14 Thread Bill Farner
/ Testing --- end-to-end tests pass (and exercise recovery tool) Thanks, Bill Farner

Review Request 64629: Use java.util.Optional throughout

2017-12-14 Thread Bill Farner
/apache/aurora/scheduler/updater/KillTaskTest.java 61f66b9ff24ef667457b07939a6cd0def88e3fd9 src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 32a8c89d3e71395696fc613da96b871330891c42 Diff: https://reviews.apache.org/r/64629/diff/1/ Testing --- Thanks, Bill

Re: Review Request 64625: Add a storage recovery tool

2017-12-15 Thread Bill Farner
will add these in a follow-up. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64625/#review193943 --- On Dec. 14, 2017

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

2017-12-13 Thread Bill Farner
.apache.org/r/64286/diff/3/?file=1914611#file1914611line117> > > > > Do we need to map edit -> op? To line up the types, yes. Same as above - unless i'm misunderstanding you. - Bill --- This is an

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

2017-12-13 Thread Bill Farner
ersion, > > ensuring snapshots will read by both fine)? > > Bill Farner wrote: > Happy to do this, i'll hold off committing until confirmed. Reporting back - i was able to run `test_http_example` from the end-to-end test on master, then with this patch, then again on master a

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

2017-12-13 Thread Bill Farner
in the test cases. src/test/python/apache/aurora/config/test_thrift.py Lines 281 (patched) <https://reviews.apache.org/r/64341/#comment272365> All of the `assert len(x) == y` checks are unnecessary now that you have the `assert set_x == set_y` checks. Thanks for adding that! - Bill

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

Review Request 63744: Remove LockStore

2017-11-11 Thread Bill Farner
1691477e8995756580162c460a82d20b82140c2d src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 4d62bba3eb64be4493dc80a1cee93159a4828c12 Diff: https://reviews.apache.org/r/63744/diff/1/ Testing --- Thanks, Bill Farner

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-11-11 Thread Bill Farner
- On Sept. 26, 2017, 1:50 p.m., Stephan Erb wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62590/ > --- > > (Updated Sept. 26, 2017, 1:50 p.m.) >

Re: Review Request 63743: Remove the internal SQL database

2017-11-11 Thread Bill Farner
mplex. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63743/#review190785 --- On Nov. 10, 2017, 8:53 p.m., Bi

Re: Review Request 63750: pants for high-sierra

2017-11-13 Thread Bill Farner
> On Nov. 13, 2017, 8:05 a.m., Bill Farner wrote: > > Stephan - FYI, i reproduce the build error: > > ``` > > Exception message: Could not satisfy all requirements for > > mesos.executor==1.4.0: > > mesos.executor==1.4.0 > > ``` > > Stephan

Re: Review Request 63750: pants for high-sierra

2017-11-13 Thread Bill Farner
: Could not satisfy all requirements for mesos.executor==1.4.0: mesos.executor==1.4.0 ``` pants.ini Lines 33-36 (original), 33-36 (patched) <https://reviews.apache.org/r/63750/#comment268413> You can now remove this workaround. - Bill Farner On Nov. 13, 2017, 4:29 a.m., se choi

Review Request 63760: Make testTaskChangedWithOldStateError more robust

2017-11-13 Thread Bill Farner
2906b08deba08f9f22bd78c8f83a78409114a913 Diff: https://reviews.apache.org/r/63760/diff/1/ Testing --- Thanks, Bill Farner

Re: Review Request 63746: Add a workaround for test_du_diskcollector failing on macOS

2017-11-13 Thread Bill Farner
`WebhookTest.testTaskChangedWithOldStateError`. I don't reproduce this locally after a few tries, but it's beginning to look like a flaky test. - Bill Farner On Nov. 12, 2017, 12:14 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 63884: Idempotent Ops and slimmed API for JobUpdateStore

2017-11-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63884/#review191580 --- Ping - Bill Farner On Nov. 16, 2017, 10:20 a.m., Bill Farner

Re: Review Request 63973: Enable custom offer scoring modules for task assignment and injecting of custom OfferManagers

2017-11-21 Thread Bill Farner
suggest removing this customization. `OfferRanker` seems like a much more reasonable level of abstraction. - Bill Farner On Nov. 20, 2017, 7:40 p.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply

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
ache.org/r/63536/ > --- > > (Updated Nov. 21, 2017, 10:42 a.m.) > > > Review request for Aurora, Jordan Ly, Santhosh Kumar Shanmugham, and Bill > Farner. > > > Repository: aurora > > > Description &g

Re: Review Request 63884: Add RemoveJobUpdates log Op, slim JobUpdateStore API

2017-11-21 Thread Bill Farner
ava 39456cfe8b8c8e198a6f8c82e09769bd75db2adc src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 661ce58a840e5a23ff5438517880877cd90d0537 Diff: https://reviews.apache.org/r/63884/diff/3/ Changes: https://reviews.apache.org/r/63884/diff/2-3/ Testing --- Thanks, Bill Farner

Re: Review Request 63884: Idempotent Ops and slimmed API for JobUpdateStore

2017-11-21 Thread Bill Farner
t jobs? You're absolutey right, this was an egregious mistake. This code will be removed in the next iteration of the patch, but thank you for catching the mistake! - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63884/#review191592 ---

Re: Review Request 63884: Idempotent Ops and slimmed API for JobUpdateStore

2017-11-21 Thread Bill Farner
----- On Nov. 16, 2017, 10:20 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63884/ >

Re: Review Request 63750: pants for high-sierra

2017-11-16 Thread Bill Farner
-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63750/#review191018 ------- On Nov. 14, 2017, 3:51 p.m., se choi wrote: > > --- &g

Re: Review Request 63884: Idempotent Ops and slimmed API for JobUpdateStore

2017-11-16 Thread Bill Farner
llerImpl`. src/test/java/org/apache/aurora/scheduler/storage/AbstractJobUpdateStoreTest.java Lines 451 (patched) <https://reviews.apache.org/r/63884/#comment268869> Whoops, i need to finish this. Coming in an updated draft. - Bill Farner On Nov. 16, 2017, 10:20 a.m., Bill Farner wrote: > > ---

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
phrase this check as: ```java assertEquals( ImmutableList.of(PENDING, ASSIGNED, ...), updatedTask.getTaskEvents().stream() .map(e -> e.getStatus()) .collect(Collectors.toList())); ``` This will give greater confidence that the

Re: Review Request 63763: Fix flaky MesosCallbackHandlerTest

2017-11-13 Thread Bill Farner
> On Nov. 13, 2017, 1:43 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java > > Line 335 (original), 337 (patched) > > <https://reviews.apache.org/r/63763/diff/1/?file=1890883#file1890883line339>

Re: Review Request 63760: Make testTaskChangedWithOldStateError more robust

2017-11-13 Thread Bill Farner
. Running again to hopefully narrow in on flaky vs persistent failure. - Bill Farner On Nov. 13, 2017, 8:32 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 63763: Fix flaky MesosCallbackHandlerTest

2017-11-13 Thread Bill Farner
/MesosCallbackHandlerTest.java Line 335 (original), 337 (patched) <https://reviews.apache.org/r/63763/#comment268440> Can we get away with `MoreExecutors.directExecutor()` instead? - Bill Farner On Nov. 13, 2017, 11:32 a.m., Jordan Ly

Re: Review Request 63744: Remove LockStore

2017-11-13 Thread Bill Farner
why the bot tried to use it again. - Bill Farner On Nov. 11, 2017, 5:42 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Review Request 63746: Add a workaround for test_du_diskcollector failing on macOS

2017-11-12 Thread Bill Farner
-pycharm-virtualenv 0f422d6baed6ce0dd77ba3b148d6ae3887028f9d src/test/python/apache/thermos/monitoring/test_disk.py e57467c61d94d67e6cf6b766c062588235b4b235 Diff: https://reviews.apache.org/r/63746/diff/1/ Testing --- Thanks, Bill Farner

Review Request 63743: Remove the internal SQL database

2017-11-10 Thread Bill Farner
/MemCronJobStoreTest.java 66b415cc395d09b15b977d0c54be94dd907a8485 src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 9e75c98dfd5fcb949cd8a3ff2030892ac3b17180 Diff: https://reviews.apache.org/r/63743/diff/1/ Testing --- Thanks, Bill Farner

Re: Review Request 63746: Add a workaround for test_du_diskcollector failing on macOS

2017-11-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63746/#review190798 --- @ReviewBot retry - Bill Farner On Nov. 12, 2017, 12:14 p.m

Re: Review Request 63564: Migrate from findbugs to spotbugs

2017-11-05 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63564/#review190110 --- Ship it! Ship It! - Bill Farner On Nov. 5, 2017, 2:42 p.m

Re: Review Request 63443: Terminate the executor on unhandled errors

2017-11-01 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63443/#review189866 --- Ship it! Ship It! - Bill Farner On Oct. 31, 2017, 9:17 a.m

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-11-09 Thread Bill Farner
> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote: > > ``` > > /bin/sh: cmake: command not found > > ``` > > > > But now i need to install cmake, so i'm not sure this pays off. > > Bill Farner wrote: > (this = the switch to cmake) > > Ste

Re: Review Request 63670: Add a test for storage durability

2017-11-09 Thread Bill Farner
ps://reviews.apache.org/r/63670/#review190462 ------- On Nov. 8, 2017, 8:13 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 63531: Update to Mesos 1.4

2017-11-09 Thread Bill Farner
rc0 has been cut, the `CHANGELOG` needs an update in this patch. If we go to rc1, i think we should pull this change in. - Bill Farner On Nov. 2, 2017, 5:03 p.m., Stephan Erb wrote: > > --- > This is an automatically generat

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
number of events stored for a task. src/main/java/org/apache/aurora/scheduler/state/StateModule.java Lines 70 (patched) <https://reviews.apache.org/r/63536/#comment268115> PubsubEventModule.bindSubscriber(binder, Partit

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
. 8, 2017, 4:48 p.m.) > > > Review request for Aurora, Jordan Ly, Santhosh Kumar Shanmugham, and Bill > Farner. > > > Repository: aurora > > > Description > --- > > This is my prototype code for adding partition-awareness to Aurora. There is &

Re: Review Request 63705: Use transition method and fix documentation in Webhooks

2017-11-09 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63705/#review190626 --- Ship it! Ship It! - Bill Farner On Nov. 9, 2017, 10:42 a.m

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) > > <https://reviews.apache.org/r/63536/diff/2/?file=1885207#file1885207line76> > > > > Thinking

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) > > <https://reviews.apache.org/r/63536/diff/2/?file=1885207#file1885207line76> > > > > Thinking

Review Request 63670: Add a test for storage durability

2017-11-08 Thread Bill Farner
/java/org/apache/aurora/scheduler/storage/log/DurableStorageTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/log/FakeLog.java PRE-CREATION Diff: https://reviews.apache.org/r/63670/diff/1/ Testing --- Thanks, Bill Farner

Re: Review Request 63685: RFC: Use new scheduler UI as landing page

2017-11-08 Thread Bill Farner
change to: ``` .put("/(?:index.html)?", "/assets/scheduler/index.html") ``` This should serve the scheduler UI when accessing `/` or `/index.html`. - Bill Farner On Nov. 8, 2017, 2:

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

2017-12-05 Thread Bill Farner
s://reviews.apache.org/r/64286/#comment271235> This was removed with no replacement, as the `Map` was replaced with a `switch`. I think we should address this with test cases for handling transaction types instead (which we do). - Bill Farner

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

2017-12-05 Thread Bill Farner
g/r/64341/#comment271239> This is irrelevant to the test case, please remove. src/test/python/apache/aurora/config/test_thrift.py Lines 236-239 (patched) <https://reviews.apache.org/r/64341/#comment271238> Please replace this with a direct equality comparison to the expected set. - Bill F

Review Request 64286: Recover snapshots via the Op stream

2017-12-05 Thread Bill Farner
883738446a093e464630d06f1241832129165fcc src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java bb0fd89b9edee2048746876da9edbaf5c9e881ca Diff: https://reviews.apache.org/r/64286/diff/1/ Testing --- end-to-end tests pass Thanks, Bill Farner

Review Request 64288: Add a SQL persistence implementation

2017-12-03 Thread Bill Farner
/schema.sql PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/sql/SqlPersistenceTest.java PRE-CREATION Diff: https://reviews.apache.org/r/64288/diff/1/ Testing --- Thanks, Bill Farner

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-03 Thread Bill Farner
- Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review192662 --- On Dec. 3, 2017, 5:09 p.m., Bill Far

Re: Review Request 63750: pants for high-sierra

2017-12-03 Thread Bill Farner
gt; 00:13:40 00:05 [requirements] > > 00:13:40 00:05 [cache] > >No cached artifacts for 35 targets. > >Invalidated 35 targets. > >Waiting for background workers to finish. > > 00

Re: Review Request 64234: Extract a storage Persistence layer

2017-12-02 Thread Bill Farner
gration utility that connects the `recover()` stream of one `Persistence` to the `persist()` of another. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64234/#review192452 --- On Nov. 30, 2017, 4:32 p.m., Bill F

Review Request 64283: Remove redundant transaction recorder

2017-12-02 Thread Bill Farner
/StreamTransaction.java a51fd18ad00537bf244419442078548d8545a841 src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java cb38f107385997b091c3b65db3a13f9cca0fa42d Diff: https://reviews.apache.org/r/64283/diff/1/ Testing --- Thanks, Bill Farner

Review Request 64359: Fix for multiple slf4j jars in intellij

2017-12-05 Thread Bill Farner
slf4j versions. Diffs - build.gradle af119910e84c48f75f2573ababcfa287c3b986fc Diff: https://reviews.apache.org/r/64359/diff/1/ Testing --- `./gradlew idea`, 'External Libraries' no longer has rogue slf4j jars, and tests don't fail as a result. Thanks, Bill Farner

Re: Review Request 64359: Fix for multiple slf4j jars in intellij

2017-12-05 Thread Bill Farner
- org.objenesis:objenesis:2.1 ``` - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64359/#review192965 -------

Review Request 64426: Remove hack for guice error logging

2017-12-07 Thread Bill Farner
--- With `103f794ed126e135f2fe0ff1bde04a4093413521`, this hack is no longer needed. Diffs - src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 7ffcf4f471b97e32426c82972472115c5c5c4d02 Diff: https://reviews.apache.org/r/64426/diff/1/ Testing --- Thanks, Bill

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-07 Thread Bill Farner
://reviews.apache.org/r/64288/diff/3-4/ Testing --- Thanks, Bill Farner

Re: Review Request 64382: Update Python deps incl requests and pex

2017-12-07 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64382/#review193145 --- Ship it! Ship It! - Bill Farner On Dec. 6, 2017, 8:40 a.m

Re: Review Request 64362: Update to guice 4.1.0, switch from jersey to resteasy

2017-12-06 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64362/#review193043 --- @ReviewBot retry - Bill Farner On Dec. 6, 2017, 6:31 a.m

Re: Review Request 64459: Deprecated Ops re-added, perform no-op instead of throwing an exception

2017-12-08 Thread Bill Farner
/DurableStorage.java Line 294 (original), 300 (patched) <https://reviews.apache.org/r/64459/#comment271843> Did the recovery with a `Lock` op fail here? If so, i'm tempted to just change this to log and avoid the extra deprecation step. - Bill Farner On Dec. 8, 2017, 12:08 p.m., Jor

Re: Review Request 64459: Deprecated Ops re-added, perform no-op instead of throwing an exception

2017-12-08 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64459/#review193313 --- Ship it! Ship It! - Bill Farner On Dec. 8, 2017, 12:08 p.m

Re: Review Request 64459: Deprecated Ops re-added, perform no-op instead of throwing an exception

2017-12-08 Thread Bill Farner
> On Dec. 8, 2017, 6:18 p.m., Bill Farner wrote: > > Ship It! > BEFORE MERGING: what is the rollback story for updates? If we upgrade to 0.20 > and then revert to 0.19, there will be no locks for in-progress updates. Will > this be an issue or was saving locks essentia

Re: Review Request 64508: Convert carriage returns to newlines in reviews

2017-12-11 Thread Bill Farner
(original), 178 (patched) <https://reviews.apache.org/r/64508/#comment272034> Can you include a comment for posterity? Referencing AURORA-1961 would be ideal since it holds context (should also be on the Bugs line of this review). - Bill Farner On Dec. 11, 2017, 1:11 p.m., Stephan Erb

Review Request 64519: Add a test to detect incompatible storage changes

2017-12-11 Thread Bill Farner
/resources/org/apache/aurora/scheduler/storage/durability/goldens/read-compatible/9-saveHostAttributes PRE-CREATION Diff: https://reviews.apache.org/r/64519/diff/1/ Testing --- Thanks, Bill Farner

Re: Review Request 64519: Add a test to detect incompatible storage changes

2017-12-11 Thread Bill Farner
ws.apache.org/r/64519/#comment272040> Here begins the golden files. These are all pretty-printed `TJSONProtocol`. While it's not easy to look at one of these and know the structure, it is fairly straightforward to map a thrift change to a golden file change. - Bill Farner On Dec. 1

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-11 Thread Bill Farner
> On Dec. 11, 2017, 6:57 p.m., Bill Farner wrote: > > I should have looked more closely at the last patch, and when preparing my > > patch before that. This test is bound to be flaky since there's nothing > > ensuring a happens-before releationship betwee

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-11 Thread Bill Farner
); +} catch (InterruptedException e) { +} errorsCounter.incrementAndGet(); LOG.error("Error sending a Webhook event", t); ``` If i get some spare time, i may take a stab at a fix; but the happens-before is what we'll want to seek. - Bill Farne

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-11 Thread Bill Farner
> On Dec. 11, 2017, 6:57 p.m., Bill Farner wrote: > > I should have looked more closely at the last patch, and when preparing my > > patch before that. This test is bound to be flaky since there's nothing > > ensuring a happens-before releationship betwee

Re: Review Request 64234: Extract a storage Persistence layer

2017-12-02 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64234/#review192634 --- @ReviewBot retry - Bill Farner On Nov. 30, 2017, 4:32 p.m

Re: Review Request 64284: Expose thrift workload stats on pruneTasks() invocations

2017-12-03 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64284/#review192646 --- Ship it! Ship It! - Bill Farner On Dec. 3, 2017, 5:20 a.m

Re: Review Request 64482: Fix flaky WebhookTest

2017-12-09 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64482/#review193339 --- Ship it! Ship It! - Bill Farner On Dec. 9, 2017, 4:08 p.m

Re: Review Request 64362: Update to guice 4.1.0, switch from jersey to resteasy

2017-12-06 Thread Bill Farner
eviews.apache.org/r/64362/#review192977 --- On Dec. 5, 2017, 11:36 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64362/ >

Review Request 64377: Remove H2-related end-to-end test case

2017-12-06 Thread Bill Farner
://reviews.apache.org/r/64377/diff/1/ Testing --- `test_kerberos_end_to_end.sh` now passes, will hold on committing until `test_end_to_end.sh` (now running) passes. Thanks, Bill Farner

Re: Review Request 64362: Update to guice 4.1.0, switch from jersey to resteasy

2017-12-06 Thread Bill Farner
before landing this patch Thanks, Bill Farner

Review Request 63401: Add support for generating patch RCs from non-master branches

2017-10-29 Thread Bill Farner
, Bill Farner

Review Request 63418: Remove endpoint.thrift, ServiceInstance is never serialized to thrift

2017-10-30 Thread Bill Farner
/apache/aurora/scheduler/http/LeaderRedirectTest.java a7cc046d97d7b817a5496a6a7190142375b7a8a1 Diff: https://reviews.apache.org/r/63418/diff/1/ Testing --- Thanks, Bill Farner

Re: Review Request 63418: Remove endpoint.thrift, ServiceInstance is never serialized to thrift

2017-10-30 Thread Bill Farner
/LeaderRedirect.java Line 88 (original), 88 (patched) <https://reviews.apache.org/r/63418/#comment266778> I've removed some superfluous field validation here (and an accompanying test case). These checks are redundant to what is performed in `Encoding#decode()`. - Bill Farner On Oct. 30, 2017, 9:

Re: Review Request 63236: Refactor veto logic to use direct method calls as opposed to pubsub events.

2017-10-24 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63236/#review189152 --- Ship it! - Bill Farner On Oct. 24, 2017, 12:10 a.m., Jordan

Re: Review Request 63282: Remove the old UI and serve the new UI instead

2017-10-24 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63282/#review189150 --- Ship it! Ship It! - Bill Farner On Oct. 24, 2017, 5:21 p.m

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-24 Thread Bill Farner
path is stressed. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review189062 --- On Oct. 18, 2017, 11:11 a.m., Bill Farner wrote: > > --- >

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-10-25 Thread Bill Farner
> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote: > > ``` > > /bin/sh: cmake: command not found > > ``` > > > > But now i need to install cmake, so i'm not sure this pays off. > > Bill Farner wrote: > (this = the switch to cmake) > > Ste

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-21 Thread Bill Farner
. Want to make sure everyone is on board with this! - Bill Farner On Oct. 18, 2017, 11:11 a.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Review Request 63202: Add test case for regression of AURORA-1952

2017-10-21 Thread Bill Farner
that fails prior to [a935389662b72486f49e507493557d360dc97178](https://github.com/apache/aurora/commit/a935389662b72486f49e507493557d360dc97178). Thanks, Bill Farner

Re: Review Request 63418: Remove endpoint.thrift, ServiceInstance is never serialized to thrift

2017-10-30 Thread Bill Farner
nd buried the field to effectively hard-code it. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63418/#review189621 -------

Review Request 63454: Use a pair of fields for caching offer resources rather than a Cache

2017-10-31 Thread Bill Farner
ops/s ``` Thanks, Bill Farner

Re: Review Request 63435: Remove inaccurate "Initializing sandbox" message

2017-10-30 Thread Bill Farner
list item. src/main/python/apache/aurora/executor/aurora_executor.py Line 98 (original), 98 (patched) <https://reviews.apache.org/r/63435/#comment266873> Bonus points for an `Awaiting task health checks` message when applicable. - Bill Farner On Oct. 30, 2017, 4:43 p.m., Stephan Erb

Re: Review Request 63199: Refactor staticallyBannedOffers into a LRU cache

2017-10-30 Thread Bill Farner
` in `OfferSettings` as the finishing touch. Nice patch! - Bill Farner On Oct. 30, 2017, 11:14 a.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 63199: Refactor staticallyBannedOffers into a LRU cache

2017-10-30 Thread Bill Farner
> On Oct. 27, 2017, 3:03 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java > > Lines 42-43 (patched) > > <https://reviews.apache.org/r/63199/diff/4/?file=1865171#file1865171line42> > > > > H

Re: Review Request 63157: Provide a formal way to disable offer declining

2017-10-19 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63157/#review188752 ------- On Oct. 19, 2017, 1:23 p.m., Bill Farner wrote: > > --- > This

<    5   6   7   8   9   10   11   >