Re: Review Request 24727: Store a lock association with job updates.

2014-08-15 Thread Bill Farner
this change? The lock is supposed to be already acquired by the > > time startJobUpdate is executed, hense the Lock instance in the argument > > set. How is it supposed to work now? > > Maxim Khutornenko wrote: > s/hense/hence > > Bill Farner wrote: >

Re: Review Request 24727: Store a lock association with job updates.

2014-08-15 Thread Bill Farner
this change? The lock is supposed to be already acquired by the > > time startJobUpdate is executed, hense the Lock instance in the argument > > set. How is it supposed to work now? > > Maxim Khutornenko wrote: > s/hense/hence > > Bill Farner wrote: >

Re: Review Request 24744: Dropping lock from startJobUpdate parameters.

2014-08-15 Thread Bill Farner
tps://reviews.apache.org/r/24744/#comment88618> We really should not expose the lock. Any attempt to do anything with the lock will ~certainly interfere with the updater. - Bill Farner On Aug. 15, 2014, 6:14 p.m., Maxim Khutornenko

Re: Review Request 24744: Dropping lock from startJobUpdate parameters.

2014-08-15 Thread Bill Farner
> On Aug. 15, 2014, 6:20 p.m., Bill Farner wrote: > > src/main/thrift/org/apache/aurora/gen/api.thrift, line 751 > > <https://reviews.apache.org/r/24744/diff/1/?file=661623#file661623line751> > > > > We really should not expose the lock. Any attempt to do

Re: Review Request 24744: Dropping lock from startJobUpdate parameters.

2014-08-15 Thread Bill Farner
> On Aug. 15, 2014, 6:20 p.m., Bill Farner wrote: > > src/main/thrift/org/apache/aurora/gen/api.thrift, line 751 > > <https://reviews.apache.org/r/24744/diff/1/?file=661623#file661623line751> > > > > We really should not expose the lock. Any attempt to do

Re: Review Request 24744: Dropping lock from startJobUpdate parameters.

2014-08-15 Thread Bill Farner
> On Aug. 15, 2014, 6:20 p.m., Bill Farner wrote: > > src/main/thrift/org/apache/aurora/gen/api.thrift, line 751 > > <https://reviews.apache.org/r/24744/diff/1/?file=661623#file661623line751> > > > > We really should not expose the lock. Any attempt to do

Re: Review Request 24744: Dropping lock from startJobUpdate parameters.

2014-08-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24744/#review50900 --- Ship it! Ship It! - Bill Farner On Aug. 15, 2014, 11:57 p.m

Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner
--- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24720/ > --- > > (Updated Aug. 15, 2014, 5:52 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. &g

Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner
hat failed to update > > and the job rollback is disabled? > > David McLaughlin wrote: > Sounds like we need another state for this. Do you have a suggestion? > INSTANCE_UPDATE_FAILED ? > > Bill Farner wrote: > INSTANCE_UPDATE_FAILED SGTM And FWIW disabled rol

Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner
hat failed to update > > and the job rollback is disabled? > > David McLaughlin wrote: > Sounds like we need another state for this. Do you have a suggestion? > INSTANCE_UPDATE_FAILED ? > > Bill Farner wrote: > INSTANCE_UPDATE_FAILED SGTM > > Bill Farner wr

Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24720/#review50911 --- Ship it! Ship It! - Bill Farner On Aug. 18, 2014, 5:49 p.m

Re: Review Request 24720: Expand actions in JobUpdateAction

2014-08-18 Thread Bill Farner
: David McLaughlin Date: Mon Aug 18 11:27:16 2014 -0700 Expand actions in JobUpdateAction Reviewed at https://reviews.apache.org/r/24720/ - Bill Farner On Aug. 18, 2014, 5:59 p.m., David McLaughlin wrote: > > --- > T

Review Request 24813: Add a job update controller interface.

2014-08-18 Thread Bill Farner
/JobUpdateControllerImpl.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/UpdateStateException.java PRE-CREATION Diff: https://reviews.apache.org/r/24813/diff/ Testing --- Thanks, Bill Farner

Re: Review Request 24813: Add a job update controller interface.

2014-08-18 Thread Bill Farner
; > > s/../. Fixed. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24813/#review50935 --- On Aug.

Re: Review Request 24813: Add a job update controller interface.

2014-08-18 Thread Bill Farner
/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/UpdateStateException.java PRE-CREATION Diff: https://reviews.apache.org/r/24813/diff/ Testing --- Thanks, Bill Farner

Review Request 24823: Use convenience function in Numbers to group instance IDs.

2014-08-18 Thread Bill Farner
build -Pq Thanks, Bill Farner

Review Request 24827: Add a pubsub event adapter to the JobUpdateController.

2014-08-18 Thread Bill Farner
a78a4d849e545000725a39fae49f406422c39da0 src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java PRE-CREATION Diff: https://reviews.apache.org/r/24827/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner

Re: Review Request 24827: Add a pubsub event adapter to the JobUpdateController.

2014-08-19 Thread Bill Farner
ally generated e-mail. To reply, visit: https://reviews.apache.org/r/24827/#review50976 ------- On Aug. 19, 2014, 12:09 a.m., Bill Farner wrote: > > --- >

Re: Review Request 24835: Implementing pause/resume/abort APIs.

2014-08-19 Thread Bill Farner
his down by changing the signature to JobUpdateController.start() to accept the lock token. - Bill Farner On Aug. 19, 2014, 1:56 a.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To r

Re: Review Request 24827: Add a pubsub event adapter to the JobUpdateController.

2014-08-19 Thread Bill Farner
/scheduler/updater/JobUpdateEventSubscriberTest.java PRE-CREATION Diff: https://reviews.apache.org/r/24827/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner

Re: Review Request 24827: Add a pubsub event adapter to the JobUpdateController.

2014-08-19 Thread Bill Farner
/scheduler/updater/JobUpdateEventSubscriberTest.java PRE-CREATION Diff: https://reviews.apache.org/r/24827/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner

Re: Review Request 24662: Implementing job update "get" thrift APIs.

2014-08-19 Thread Bill Farner
che.org/r/24662/#comment88890> ditto - Bill Farner On Aug. 14, 2014, 8:20 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 24915: Adding initial GC task delay on scheduler restart.

2014-09-02 Thread Bill Farner
ified. An immediate issue i have with this patch is that it introduces a memory leak (in practice, this is hidden with default settings due to failover failover hides this). I'll reply on the ticket to try to reach consensus on what the problem is and what it means to fix it. - Bill Farn

Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Bill Farner
urora/scheduler/app/local/simulator/FakeSlaves.java PRE-CREATION Diff: https://reviews.apache.org/r/25257/diff/ Testing --- ./gradlew run, also ran directly in intellij Thanks, Bill Farner

Re: Review Request 24752: combine finalization_wait when combining tasks

2014-09-02 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24752/#review52088 --- Ping, Brian? - Bill Farner On Aug. 28, 2014, 3:27 p.m., Matthew

Re: Review Request 24815: Refactoring SchedulerCore final part.

2014-09-02 Thread Bill Farner
ment90835> Is this interface useful? The implementation logic is trivial, and it's only used from SchedulerThriftInterface. I suggest putting it there. - Bill Farner On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote: > > -

Re: Review Request 25158: Aurora Update UI

2014-09-02 Thread Bill Farner
reviews.apache.org/r/25158/#review51808 --- On Aug. 28, 2014, 6:13 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25158/ > ---

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Bill Farner
frequently we will send resource offers from the fake master to the scheduler. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25257/#review52050 --------

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-02 Thread Bill Farner
PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java PRE-CREATION Diff: https://reviews.apache.org/r/25257/diff/ Testing --- ./gradlew run, also ran directly in intellij Thanks, Bill Farner

Review Request 25285: Upgrade to latest in jetty 7.x series.

2014-09-02 Thread Bill Farner
page work. Thanks, Bill Farner

Review Request 25300: Add a state machine to react to job update status changes.

2014-09-03 Thread Bill Farner
Diff: https://reviews.apache.org/r/25300/diff/ Testing --- `./gradlew build -Pq` Thanks, Bill Farner

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-03 Thread Bill Farner
ropped now? Sure, done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25257/#review52118 --- On Sept. 2, 20

Re: Review Request 25257: Add a separate main class that runs the scheduler in local mode.

2014-09-03 Thread Bill Farner
PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/local/simulator/FakeSlaves.java PRE-CREATION Diff: https://reviews.apache.org/r/25257/diff/ Testing --- ./gradlew run, also ran directly in intellij Thanks, Bill Farner

Re: Review Request 25300: Add a state machine to react to job update status changes.

2014-09-03 Thread Bill Farner
-mail. To reply, visit: https://reviews.apache.org/r/25300/#review52195 --- On Sept. 3, 2014, 4:02 p.m., Bill Farner wrote: > > --- > This is an autom

Re: Review Request 25300: Add a state machine to react to job update status changes.

2014-09-03 Thread Bill Farner
lso throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED > > -> ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift > > APIs (i.e. pause/resume/abort). Would it make sense to alter > > ALLOWED_TRANSITIONS with same state no-op transitions or throw here onl

Re: Review Request 25300: Add a state machine to react to job update status changes.

2014-09-03 Thread Bill Farner
lso throw for in-place transitions (e.g. ROLL_FORWARD_PAUSED > > -> ROLL_FORWARD_PAUSED) making it harder to implement idempotent thrift > > APIs (i.e. pause/resume/abort). Would it make sense to alter > > ALLOWED_TRANSITIONS with same state no-op transitions or throw here onl

Re: Review Request 25311: Checking for cron jobs in startJobUpdate()

2014-09-03 Thread Bill Farner
/SchedulerThriftInterface.java <https://reviews.apache.org/r/25311/#comment90973> "Cron jobs may only be updated by calling replaceCronTemplate." - Bill Farner On Sept. 3, 2014, 7:41 p.m., Maxim Khutornenko wrote: > > --

Re: Review Request 24915: Adding initial GC task delay on scheduler restart.

2014-09-04 Thread Bill Farner
> On Sept. 2, 2014, 4:43 p.m., Bill Farner wrote: > > I'm concerned that the problem we're solving is underspecified. An > > immediate issue i have with this patch is that it introduces a memory leak > > (in practice, this is hidden with default settings due

Re: Review Request 24915: Adding initial GC task delay on scheduler restart.

2014-09-04 Thread Bill Farner
d launch a GC task - Bill Farner On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://revi

Re: Review Request 25285: Upgrade to latest in jetty 7.x series.

2014-09-04 Thread Bill Farner
rks, and all pages linked from the index page work. Thanks, Bill Farner

Re: Review Request 25285: Upgrade to latest in jetty 7.x series.

2014-09-04 Thread Bill Farner
Testing --- ./gradlew build -Pq Ran the scheduler in vagrant, verified index page works, request logging works, and all pages linked from the index page work. Thanks, Bill Farner

Review Request 25346: Specify required vagrant version in Vagrantfile.

2014-09-04 Thread Bill Farner
://issues.apache.org/jira/browse/AURORA-683 Repository: aurora Description --- Specify required vagrant version in Vagrantfile. Diffs - Vagrantfile ea0b25294f0efeec745dc8612e4d7d02979079cc Diff: https://reviews.apache.org/r/25346/diff/ Testing --- vagant up Thanks, Bill Farner

Re: Review Request 25350: Fix build problem.

2014-09-04 Thread Bill Farner
specific? This isn't very friendly when scanning the commit log. src/test/python/apache/aurora/client/cli/test_status.py <https://reviews.apache.org/r/25350/#comment91105> TODO inject a fake clock? - Bill Farner On Sept. 4, 2014, 7:45 p.m., Mark Chu-Ca

Re: Review Request 24815: Refactoring SchedulerCore final part.

2014-09-04 Thread Bill Farner
> On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, > > line 35 > > <https://reviews.apache.org/r/24815/diff/1/?file=662758#file662758line35> > > > > Is this interface usefu

Re: Review Request 24815: Refactoring SchedulerCore final part.

2014-09-04 Thread Bill Farner
> On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, > > line 35 > > <https://reviews.apache.org/r/24815/diff/1/?file=662758#file662758line35> > > > > Is this interface usefu

Re: Review Request 24815: Refactoring SchedulerCore final part.

2014-09-04 Thread Bill Farner
> On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java, line > > 120 > > <https://reviews.apache.org/r/24815/diff/1/?file=662756#file662756line120> > > > > At first glance, thi

Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.

2014-09-04 Thread Bill Farner
adlew build -Pq Thanks, Bill Farner

Re: Review Request 24915: Adding initial GC task delay on scheduler restart.

2014-09-04 Thread Bill Farner
> On Sept. 4, 2014, 5:44 p.m., Bill Farner wrote: > > Per offline discussion, i've realized my memory leak concern is ~moot since > > we already have the unbounded HostAttributes store. I liked your > > suggestion of augmenting the way this soft state is used: >

Re: Review Request 24915: Adding initial GC task delay on scheduler restart.

2014-09-04 Thread Bill Farner
pse, and the new GcExecutorSettings method can go away. src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java <https://reviews.apache.org/r/24915/#comment91148> Please wrap this with Collections.synchronizedMap to preserve thread safety. - Bill Farner On Aug. 20, 2014, 11:35 p

Re: Review Request 25357: Adding support for per-job task status metrics.

2014-09-04 Thread Bill Farner
tps://reviews.apache.org/r/25357/#comment91162> To mitigate the risk of OOM in the scheduler's internal TSDB, we should use untracked stats, similar to what's done in MetricCalculator.java. - Bill Farner On Sept. 4, 2014, 9:42 p.m., Maxim K

Re: Review Request 25357: Adding support for per-job task status metrics.

2014-09-05 Thread Bill Farner
tps://reviews.apache.org/r/25357/#comment91236> I should have been more explicit. I would prefer that only the job-scoped vars be `untracked()`. The others are bounded, and useful to have tracked. - Bill Farner On Sept. 5, 2014, 12:23 a.m., Maxim Khutornenko

Re: Review Request 25366: Set principal field in FrameworkInfo struct.

2014-09-05 Thread Bill Farner
to exercise this code? - Bill Farner On Sept. 4, 2014, 11:36 p.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 24815: Refactoring SchedulerCore final part.

2014-09-05 Thread Bill Farner
here: https://issues.apache.org/jira/browse/AURORA-423 src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java <https://reviews.apache.org/r/24815/#comment91242> s/ throws Exception//? - Bill Farner On

Re: Review Request 25391: Add information about log initialisation step.

2014-09-05 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25391/#review52468 --- Ship it! Ship It! - Bill Farner On Sept. 5, 2014, 3:45 p.m

Re: Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.

2014-09-05 Thread Bill Farner
t: https://reviews.apache.org/r/25356/#review52363 --- On Sept. 4, 2014, 9:19 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: &

Re: Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.

2014-09-05 Thread Bill Farner
e.org/r/25356/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner

Re: Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.

2014-09-05 Thread Bill Farner
the > > current implementation where a disjoint --shards value fails the update. > > Bill Farner wrote: > I think it's best for that to happen higher in the stack. The thrift > interface is probably the best place for this, to validate a > JobUpdateConfiguration whe

Re: Review Request 25357: Adding support for per-job task status metrics.

2014-09-05 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/#review52497 --- Ship it! Ship It! - Bill Farner On Sept. 5, 2014, 6:47 p.m

Re: Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.

2014-09-05 Thread Bill Farner
e.org/r/25356/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner

Re: Review Request 24815: Refactoring SchedulerCore final part.

2014-09-05 Thread Bill Farner
> On Sept. 5, 2014, 3:25 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, > > line 47 > > <https://reviews.apache.org/r/24815/diff/2/?file=679138#file679138line47> > > > > s/instances/newInstances/

Re: Review Request 25319: Tweak contributors doc to add details on finding newbie issues.

2014-09-05 Thread Bill Farner
319/#comment91312> shorten this to line up with the heading - Bill Farner On Sept. 3, 2014, 11:41 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 25361: Update vagrant provisioning and aurorabuild scripts to remove the need to sudo to run build commands in the vagrant image.

2014-09-05 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25361/#review52512 --- Ship it! Ship It! - Bill Farner On Sept. 4, 2014, 10:54 p.m

Re: Review Request 25365: Update slave url to use the thermos port.

2014-09-05 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25365/#review52513 --- Ship it! Ship It! - Bill Farner On Sept. 4, 2014, 11:09 p.m

Re: Review Request 25398: Make the offer hold jitter window configurable.

2014-09-05 Thread Bill Farner
? You'll need a way to plumb in your own Random as well. - Bill Farner On Sept. 5, 2014, 9:30 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 25361: Update vagrant provisioning and aurorabuild scripts to remove the need to sudo to run build commands in the vagrant image.

2014-09-05 Thread Bill Farner
> On Sept. 5, 2014, 9:40 p.m., Bill Farner wrote: > > Ship It! This is now on master ``` commit 599a7dc Author: Joshua Cohen Date: Fri Sep 5 14:40:46 2014 -0700 Update vagrant provisioning and aurorabuild scripts to remove the need to sudo to run build commands in the vagr

Re: Review Request 25365: Update slave url to use the thermos port.

2014-09-05 Thread Bill Farner
> On Sept. 5, 2014, 9:41 p.m., Bill Farner wrote: > > Ship It! Thanks! This is now on master ``` commit 277103b Author: Joshua Cohen Date: Fri Sep 5 15:13:33 2014 -0700 Update slave url to use the thermos port. Bugs closed: AURORA-644 Reviewed

Re: Review Request 25319: Tweak contributors doc to add details on finding newbie issues.

2014-09-05 Thread Bill Farner
: Joshua Cohen Date: Fri Sep 5 15:14:58 2014 -0700 Tweak contributors doc to add details on finding newbie issues. Reviewed at https://reviews.apache.org/r/25319/ ``` - Bill Farner On Sept. 5, 2014, 9:43 p.m., Joshua Cohen wrote

Re: Review Request 25455: Deprecating PopulateJobResult.populated thrift field.

2014-09-08 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25455/#review52671 --- Ship it! - Bill Farner On Sept. 8, 2014, 9:46 p.m., Maxim

Re: Review Request 25398: Make the offer hold jitter window configurable.

2014-09-09 Thread Bill Farner
/RandomJitterReturnDelayTest.java <https://reviews.apache.org/r/25398/#comment91720> Consider using the mock throughout. You get a little extra bit of verification that it is not invoked. - Bill Farner On Sept. 8, 2014, 9:09 p.m., Joshua Cohen

Re: Review Request 25483: Transaction all SQL under MemStorage

2014-09-09 Thread Bill Farner
ra/scheduler/storage/mem/MemStorageTest.java <https://reviews.apache.org/r/25483/#comment91910> assertEquals(ImmutableMap.<..>of(), fetchQuotas()); In this case it's not _that_ helpful, but this pattern results in generally more useful output from junit (expected vs actua

Re: Review Request 25483: Transaction all SQL under MemStorage

2014-09-09 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25483/#review52797 --- Ship it! Ship It! - Bill Farner On Sept. 9, 2014, 9:05 p.m

Re: Review Request 25398: Make the offer hold jitter window configurable.

2014-09-10 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25398/#review52958 --- Ship it! Ship It! - Bill Farner On Sept. 9, 2014, 4:54 p.m

Re: Review Request 25398: Make the offer hold jitter window configurable.

2014-09-10 Thread Bill Farner
> On Sept. 10, 2014, 10:15 p.m., Bill Farner wrote: > > Ship It! Thanks! This is now on master: ``` commit cc70136 Author: Joshua Cohen Date: Wed Sep 10 15:38:56 2014 -0700 Make the offer hold jitter window configurable. Bugs closed: AURORA-313 Reviewed

Review Request 25529: Add a controller for job updates.

2014-09-10 Thread Bill Farner
che/aurora/scheduler/updater/strategy/QueueStrategyTest.java f9a2806708ea1a06a4f0ebe118c777fd9dbe2dcc Diff: https://reviews.apache.org/r/25529/diff/ Testing --- ./gradlew build -Pq Jacoco reports 98% line coverage, 96% branch coverage for the updater package. Thanks, Bill Farner

Re: Review Request 25481: Adding JobUpdateRequest validation.

2014-09-11 Thread Bill Farner
an internal error (based on LoggingInterceptor, which handles uncaught exceptions). - Bill Farner On Sept. 9, 2014, 7:46 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To r

Re: Review Request 25481: Adding JobUpdateRequest validation.

2014-09-11 Thread Bill Farner
> On Sept. 11, 2014, 5:54 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 1371 > > <https://reviews.apache.org/r/25481/diff/1/?file=684118#file684118line1371> > > > > I believe t

Re: Review Request 25548: Apply GzipFilter to POSTs as well as GETs

2014-09-11 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25548/#review53084 --- Ship it! Ship It! - Bill Farner On Sept. 11, 2014, 6:20 p.m

Re: Review Request 25556: Dropping synchronized from validateIfLocked()

2014-09-11 Thread Bill Farner
regression shouldn't be too hard, but let's get the fix in. - Bill Farner On Sept. 11, 2014, 9:08 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://revi

Re: Review Request 25556: Dropping synchronized from validateIfLocked()

2014-09-11 Thread Bill Farner
https://github.com/wfarner/incubator-aurora/commit/a62f794c6052f6abfa53197dede7f6d21f80f4e4 - Bill Farner On Sept. 11, 2014, 9:08 p.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 25529: Add a controller for job updates.

2014-09-11 Thread Bill Farner
che.org/r/25529/diff/1/?file=685018#file685018line93> > > > > How are these errors going to be surfaced to the user? Maxim and i stepped on toes here, so this validation moves up the stack with https://reviews.apache.org/r/25481/ - Bill ------

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
e a bad idea to encapsulate this into a InstanceName class > > that contains the logic for constructing one from JobKey + instanceId and > > how to render it to a String? > > > > It seems like a bit of overkill but I dislike the + "/" in the code. > > Bill

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
e addition of additional statuses in the future > > rendering that assumption invalid?) I've refactored this to eliminate the fall-through branch. Thanks for the nudge! - Bill --- This is an automatically generated e

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
a/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java > f38e6a3e6a798b1c78d73c6d19404156eb6d8c91 > > src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java > 41274421aaae30b43abc3da15a63276a941094f9 > > src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java > 6eed641895123d21ed760fa755ce7b30cec3fd44 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdateControllerTest.java > f0b68350ea39e5925a911e46532982c3d61ee0d6 > > src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactoryImplTest.java > ae654625370fd3ba8be59b9a37ba343ec7115cba > > src/test/java/org/apache/aurora/scheduler/updater/strategy/BatchStrategyTest.java > e6742dcbe101389710c11e9b75a508f7b61ffe49 > > src/test/java/org/apache/aurora/scheduler/updater/strategy/QueueStrategyTest.java > f9a2806708ea1a06a4f0ebe118c777fd9dbe2dcc > > Diff: https://reviews.apache.org/r/25529/diff/ > > > Testing > --- > > ./gradlew build -Pq > > Jacoco reports 98% line coverage, 96% branch coverage for the updater package. > > > Thanks, > > Bill Farner > >

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
be2dcc Diff: https://reviews.apache.org/r/25529/diff/ Testing (updated) --- ./gradlew build -Pq Jacoco reports 100% line coverage, 98% branch coverage for the updater package. Thanks, Bill Farner

Re: Review Request 25459: Adding quota check into startJobUpdate.

2014-09-12 Thread Bill Farner
ren't baffled when they're being charged for a paused update. - Bill Farner On Sept. 9, 2014, 12:30 a.m., Maxim Khutornenko wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25593/#review53243 --- Ship it! Ship It! - Bill Farner On Sept. 12, 2014, 9:43 p.m

Re: Review Request 25259: Add update information to the scheduler UI

2014-09-12 Thread Bill Farner
owed w.r.t. newlines here? I thought i had the style figured out until here. - Bill Farner On Sept. 12, 2014, 10:39 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To r

Re: Review Request 25255: Implement server-driven update commands.

2014-09-15 Thread Bill Farner
In the world of 'nouns' and 'verbs', i imagine a user would think "i want to update my job", not "i want to start an update". So if the goal of organizing commands by nouns is to make it intuitive, i think "update start" is less natural than "job update". -=Bill On Mon, Sep 15, 2014 at 10:25 AM,

Re: Review Request 25259: Add update information to the scheduler UI

2014-09-15 Thread Bill Farner
> On Sept. 12, 2014, 11:47 p.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html, > > line 17 > > <https://reviews.apache.org/r/25259/diff/5/?file=688118#file688118line17> > > > > I'd love to see U

Re: Review Request 25653: Added store APIs to query for IJobUpdate and IJobUpdateConfiguration.

2014-09-15 Thread Bill Farner
/JobUpdateStore.java <https://reviews.apache.org/r/25653/#comment93027> s/of/of a/ src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java <https://reviews.apache.org/r/25653/#comment93028> s/of/of a/ - Bill Farner On Sept. 15, 2014, 6:29 p.m., Maxim Khuto

Re: Review Request 25529: Add a controller for job updates.

2014-09-15 Thread Bill Farner
che/aurora/scheduler/updater/strategy/QueueStrategyTest.java f9a2806708ea1a06a4f0ebe118c777fd9dbe2dcc Diff: https://reviews.apache.org/r/25529/diff/ Testing --- ./gradlew build -Pq Jacoco reports 100% line coverage, 98% branch coverage for the updater package. Thanks, Bill Farner

Re: Review Request 25673: Fixing broken test.

2014-09-15 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25673/#review53428 --- Ship it! Ship It! - Bill Farner On Sept. 15, 2014, 10:56 p.m

Review Request 25671: Instruct thrift to generate private fields in java.

2014-09-15 Thread Bill Farner
f100ada567e3a0945af00c78c84f72c86e1bfb99 Diff: https://reviews.apache.org/r/25671/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner

Re: Review Request 25529: Add a controller for job updates.

2014-09-15 Thread Bill Farner
ome redundancy, and less certainty that the gates are in place. I'd also like to see if the TODO about moving the logic to SchedulerThriftInterface pans out. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/#review53425

Re: Review Request 25529: Add a controller for job updates.

2014-09-15 Thread Bill Farner
ept. 15, 2014, 11:13 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, > > line 86 > > <https://reviews.apache.org/r/25529/diff/3/?file=689760#file689760line86> > > > > redundant use of toString

Re: Review Request 25529: Add a controller for job updates.

2014-09-15 Thread Bill Farner
> line 95 > > <https://reviews.apache.org/r/25529/diff/3/?file=689756#file689756line95> > > > > Is this just Guava Service#start()? Maybe implement that interface (by > > extending AbstractIdleService) here instead so that we get state monitoring > > and

Re: Review Request 25529: Add a controller for job updates.

2014-09-15 Thread Bill Farner
ler/updater/strategy/QueueStrategyTest.java f9a2806708ea1a06a4f0ebe118c777fd9dbe2dcc Diff: https://reviews.apache.org/r/25529/diff/ Testing --- ./gradlew build -Pq Jacoco reports 100% line coverage, 98% branch coverage for the updater package. Thanks, Bill Farner

Re: Review Request 25255: Implement server-driven update commands.

2014-09-16 Thread Bill Farner
be exposed until we have all the required code in place. With that assumption, and the assumption that we are not bound to these nouns/verbs long-term, i'm willing to skip the naming discussion. - Bill Farner On Sept. 11, 2014, 2:55 p.m., Mark Chu-Carroll

Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.

2014-09-16 Thread Bill Farner
d -Pq Thanks, Bill Farner

Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.

2014-09-16 Thread Bill Farner
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53585 --- On Sept. 16, 2014, 8:37 p.m., Bill Farner wrote: > > --- > This is an a

  1   2   3   4   5   6   7   8   9   10   >