Re: Review Request 32352: Making preemptor asynchronous. Part 3(final) - background service.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/#review77609 --- src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java https://reviews.apache.org/r/32352/#comment125743 Isn't this scenario transparent to the scheduler, though? If i understand you correctly, an available reservation means the slot has been freed and is now a reserved Offer, which the assigner function handles. However, another nice TODO would be to eventually remove the fallback to preemption here. Seems reasonable that this body of code only speaks in Offers, and the newly-established territory is responsible for generating offers from tasks when it sees fit. src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java https://reviews.apache.org/r/32352/#comment125744 I'm beginning to question this pattern. It seems reasonable to insist that the global injector _never_ has a binding for a general type like this. With that in mind, you should be comfortable using a private module and an unqualified binding, saving this kind of clutter. What do you think? src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java https://reviews.apache.org/r/32352/#comment125747 This regresses on some important characteristics of `TaskGroups`: - starvation avoidance: a very large job (even a low priority one) can starve a single restarting instance - backoffs: we will perform redundant and possibly hopeless work to try to schedule the same things without reducing frequency It might be easier to retrofit `TaskGroups` to allow injection of the underlying work than to reimplement those behaviors, but i'm interested in what you think. - Bill Farner On March 21, 2015, 2:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/ --- (Updated March 21, 2015, 2:19 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Summary of changes: - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction. - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots. - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots. - preemption_delay is reduced from 10 to 3 minutes. Benchmark refactoring will be addressed separately. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 Diff: https://reviews.apache.org/r/32352/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/#review77448 --- ~All minor stuff. config/pmd/custom.xml https://reviews.apache.org/r/32329/#comment125547 Please add a comment explaining the use case we're advocating with this. src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java https://reviews.apache.org/r/32329/#comment125504 Seems like this value has meaning. It might be worth extracting a constant and document how it ties in with other components/configuration. src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java https://reviews.apache.org/r/32329/#comment125506 We don't do this in other interceptors. Seems like this is trading a NullPointerException for an IllegalStateException. I don't feel strongly, but i also wouldn't be upset if this code disappeared. src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java https://reviews.apache.org/r/32329/#comment125505 remove newline src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java https://reviews.apache.org/r/32329/#comment125507 This somewhat mirrors how authorization is done in some RPCs today, but i could imagine this being a surprise doen the road - that you have access to all affected jobs, but are denied. We briefly discussed this offline, but if you changed StructGetter to return `SetV` instead of `OptionalV`, you can restore expected behavior. src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java https://reviews.apache.org/r/32329/#comment125545 A doc would be helpful here. At first glance, it's odd that one method can produce multiple 'candidate methods'. src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java https://reviews.apache.org/r/32329/#comment125543 Added protection - filter and throw if there's != 1 annotated parameter. src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java https://reviews.apache.org/r/32329/#comment125542 Skip the Optional dance and push the throw up to annotatedParameterIndex. src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java https://reviews.apache.org/r/32329/#comment125546 Maybe a better message is No FieldGetter was supplied for x src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java https://reviews.apache.org/r/32329/#comment125512 Consider s/Struct/Thrift/ src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java https://reviews.apache.org/r/32329/#comment125509 `Struct` doesn't seem like the right noun here. Perhaps `Field`? src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java https://reviews.apache.org/r/32329/#comment125508 I don't see any areas where this is used as a `Function`. If we're not getting anything from extending Function, i suggest you declare the method here and not extend. src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java https://reviews.apache.org/r/32329/#comment125510 Is the coupling to TBase necessary here? src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java https://reviews.apache.org/r/32329/#comment125511 Ditto. - Bill Farner On March 23, 2015, 7:14 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- (Updated March 23, 2015, 7:14 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Diffs - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora
Re: Review Request 32377: Add a mechanism to lazily instantiate module classes.
On March 23, 2015, 5:43 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/app/Modules.java, line 99 https://reviews.apache.org/r/32377/diff/1/?file=902301#file902301line99 Can you push this to ModuleParser? Good call, done. On March 23, 2015, 5:43 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java, line 61 https://reviews.apache.org/r/32377/diff/1/?file=902302#file902302line61 Can you push the lazy instantiation logic to ModuleParser and change this back to ArgSetModule? Kevin Sweeney wrote: To be clear I suggest defining ```java private static final ArgSetModule SHIRO_REALM_MODULES = Arg.create(ImmutableSet.of(Modules.lazilyInstantiated(IniShiroRealmModule.class))); ``` Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32377/#review77423 --- On March 22, 2015, 6:37 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32377/ --- (Updated March 22, 2015, 6:37 p.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1217 https://issues.apache.org/jira/browse/AURORA-1217 Repository: aurora Description --- Add an indirection for modules that must not be statically instantiated. Diffs - src/main/java/org/apache/aurora/scheduler/app/Modules.java e95cb2a255e6986e0678a4085036deb24f9cb359 src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32377/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32377: Add a mechanism to lazily instantiate module classes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32377/ --- (Updated March 23, 2015, 6:51 p.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1217 https://issues.apache.org/jira/browse/AURORA-1217 Repository: aurora Description --- Add an indirection for modules that must not be statically instantiated. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/app/Modules.java e95cb2a255e6986e0678a4085036deb24f9cb359 src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java fe6409ab14ebfb89b161f919f764879d42e53877 src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32377/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32329: Extract job key from RPC parameters
On March 23, 2015, 8:59 p.m., Joshua Cohen wrote:
Thanks, this is already much easier to follow.
One general question on the overall approach: do you think the DRY benefits
of using composed `StructFieldGetter`s to generate the functions that allow
walking from the starting type to `JobKey` outweigh the readability
improvements of using a fully explicit mapping? I.e.
FunctionJobUpdateRequest, JobKey UPDATE_REQUEST_TO_JOB_KEY = new
Function... {
@Override
public JobKey apply(JobUpdateRequest request) {
return request.getTaskConfig().getJobKey();
}
};
I raised this question offline as well. It's not clear to me that this
DRY-ness is worth the complexity. Required null checking in the call chain is
one downside to the more direct approach you illustrated.
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32329/#review77289
---
On March 23, 2015, 7:14 p.m., Kevin Sweeney wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32329/
---
(Updated March 23, 2015, 7:14 p.m.)
Review request for Aurora, Joshua Cohen and Bill Farner.
Bugs: AURORA-1187
https://issues.apache.org/jira/browse/AURORA-1187
Repository: aurora
Description
---
Apologies for the large diff, this wound up needing to input validation at
the AOP layer.
Probably the best place to start reading this diff is ApiSecurityIT to see
the feature this patch enables.
Diffs
-
config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb
src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java
827e85b6cac8bd52359610bbc2002973a769705c
src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
2408cd1f9af5f109a339f5c78134465cb117f7fc
src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
cc9cfd38239f909b8a77bd1a773e31ec30130d41
src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java
808987939b2c4a850e488dc033b50b0178e95ba0
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java
4e341e05c34b1be38f0040c26b671a0cc797a771
src/main/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetter.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetter.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/api/security/StructGetters.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
5588d1793d6713ee4581ac9f938d9a8689acb315
src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java
bdd2185f3a7a94b39bcec3c73455e970d87f0c6a
src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java
cafd10f6b705568588c1b92644b482003242fe2e
src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
ed284f46ac8f01bd6d9e317f995f16d6e666a68d
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
76cb691e6d7d4fada3a18fde73aceed7039bcaa4
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java
d2ba2730c4509dc9a636fd32e9244b0d7fa2884f
src/test/java/org/apache/aurora/scheduler/http/api/security/StructFieldGetterTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
1f24e7d47e1f777ffef19a73d01171fcacd31cdb
src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java
d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b
Diff: https://reviews.apache.org/r/32329/diff/
Testing
---
./gradlew -Pq build
Thanks,
Kevin Sweeney
Re: Review Request 32377: Add a mechanism to lazily instantiate module classes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32377/ --- (Updated March 24, 2015, 12:31 a.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1217 https://issues.apache.org/jira/browse/AURORA-1217 Repository: aurora Description --- Add an indirection for modules that must not be statically instantiated. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/app/Modules.java e95cb2a255e6986e0678a4085036deb24f9cb359 src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java fe6409ab14ebfb89b161f919f764879d42e53877 src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32377/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32377: Add a mechanism to lazily instantiate module classes.
On March 23, 2015, 6:56 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/app/Modules.java, line 99 https://reviews.apache.org/r/32377/diff/2/?file=903369#file903369line99 consider adding a Class? extends Module... overload. -1 on the overload, +1 on the switch to that as the only implementation (since it's the only use case). - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32377/#review77447 --- On March 23, 2015, 6:51 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32377/ --- (Updated March 23, 2015, 6:51 p.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1217 https://issues.apache.org/jira/browse/AURORA-1217 Repository: aurora Description --- Add an indirection for modules that must not be statically instantiated. Diffs - src/main/java/org/apache/aurora/scheduler/app/Modules.java e95cb2a255e6986e0678a4085036deb24f9cb359 src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java fe6409ab14ebfb89b161f919f764879d42e53877 src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32377/diff/ Testing --- Thanks, Bill Farner
Review Request 32377: Add a mechanism to lazily instantiate module classes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32377/ --- Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1217 https://issues.apache.org/jira/browse/AURORA-1217 Repository: aurora Description --- Add an indirection for modules that must not be statically instantiated. Diffs - src/main/java/org/apache/aurora/scheduler/app/Modules.java e95cb2a255e6986e0678a4085036deb24f9cb359 src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java cc9cfd38239f909b8a77bd1a773e31ec30130d41 src/test/java/org/apache/aurora/scheduler/app/ModulesTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32377/diff/ Testing --- Thanks, Bill Farner
Review Request 32369: Simplify port name association.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32369/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- I realized that `StateManagerImpl` had an unnecessarily-complex function to associate assigned ports with port names, with error checking along the way. I have simplified this function and relocated it closer to where ports are assigned. Diffs - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 93fc05ef8b4cad187b1d36d05d2b4bb509ed60e7 src/main/java/org/apache/aurora/scheduler/state/StateManager.java 50ff4ec915352ead8c03f494f38522bcdeca3617 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 8721466b935da22ac9dd491b06f5e7ddc7f913e1 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java c44ff339b94cac20f4fb7e69a8e403fd63c132ca src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 17b170261b87506078d5463a5ed55fbc1cd8 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 06a19038f99f88d28c5548055bd82b0aebb461ac Diff: https://reviews.apache.org/r/32369/diff/ Testing --- Thanks, Bill Farner
Review Request 32371: Refine types used in QuotaManager, share more functions/predicates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32371/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- I found myself in here while doing some exploratory changes to `TaskStore`. I started by using `IAssignedTask` and `ITaskConfig` where possible, rather than `IScheduledTask`, and that snowballed into a refactor to extract common functions and predicates. Feel free to push back if you feel this reduces readability or seems like code shuffling with little gain. One upside to this outcome is the imminent post-JDK8 lambda refactor will cause this diff to be a net reduction in code. Diffs - src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 39e930c4bfe716765908c6d78b58c831b6fb341b src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java b53086169aa53d27a39a01cadf8d3c4a8ecb68de src/main/java/org/apache/aurora/scheduler/updater/Updates.java 776278cb64c7ce0419a692143e458801e3b1c37f Diff: https://reviews.apache.org/r/32371/diff/ Testing --- Thanks, Bill Farner
Review Request 32372: Clean up bindings in DbModule.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32372/ --- Review request for Aurora and David McLaughlin. Repository: aurora Description --- Use more of the built-in facilities provided by `MyBatisModule`. Diffs - src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2a75646867a5af246625f7cf3910fd9a1bbf4f03 Diff: https://reviews.apache.org/r/32372/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32353: Renaming PreemptionSlotFinder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/#review77317 --- Ship it! Ship It! - Bill Farner On March 21, 2015, 12:25 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/ --- (Updated March 21, 2015, 12:25 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- IDE-driven renaming. Prerequisite for the final preemptor refactoring step. Diffs - src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 Diff: https://reviews.apache.org/r/32353/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32231: AURORA-1189: Adding check to see if java version is below 1.8
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32231/#review77222 --- Ship it! Cool, spun this up in vagrant and confirmed: ``` W0320 15:08:49.773 THREAD1 org.apache.aurora.scheduler.app.SchedulerMain.run: ** * * * Beginning with Aurora 0.9.0, you'll need Java 1.8 to run aurora! * Currently you're running 1.7.0_75 * * ** ``` Just one nit, i'll put this on master after the diff is updated. src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java https://reviews.apache.org/r/32231/#comment125098 Remove this, the code below stands well enough on its own. - Bill Farner On March 19, 2015, 9:36 p.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32231/ --- (Updated March 19, 2015, 9:36 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1189 https://issues.apache.org/jira/browse/AURORA-1189 Repository: aurora Description --- AURORA-1189: Adding check to see if java version is below 1.8 I think it should be fine to check the java version that naive (comparing the char at 3rd posion of java.version) since this check will probably already be removed again until java 1.10 comes out. Diffs - src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 66fb432028c26ea9ed8da475da6c3cbee5e535fc Diff: https://reviews.apache.org/r/32231/diff/ Testing --- * ./gradlew test * running scheduler on vagrant machine to check if log message appears * couldn't get the scheduler run properly on java 1.8 to check if the message doesn't appear there Thanks, Florian Pfeiffer
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77242 --- Realized i neglected to update docs. Please review what's here, new patch will come shortly with doc updates. - Bill Farner On March 20, 2015, 5:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 5:26 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
On March 20, 2015, 5:59 p.m., Zameer Manji wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, line 101 https://reviews.apache.org/r/32313/diff/1/?file=901501#file901501line101 Using a raw mock here is a little bit dangerous if the shape of the raw config changes underneath the test. Would you mind setting the spec argument of the Mock class here? http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock I was thinking of something like Mock(spec=['has_cron_schedule']). Alternatively you could set the spec to be the Job schema object. I think it could be of the form of Mock(spec=aurora.config.schema.base.Job) or Mock(spec=aurora.config.schema.base.Job()). This might be impossible because Job() is a pystachio object but I strongly suggest investigating it. This way if we try to access other attributes the test will fail. I ventured down this path, but it appears that pystachio is too dynamic for this to be possible [1]. I'll do the spec_set routine, but i fear this will be untenable in other scenarios. [1] https://github.com/wickman/pystachio/blob/master/pystachio/composite.py#L205-L211 - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77248 --- On March 20, 2015, 5:26 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 5:26 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 7:51 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32323: DRY up PMD configuration.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32323/#review77261 --- Ship it! Thanks! Good find! - Bill Farner On March 20, 2015, 6:56 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32323/ --- (Updated March 20, 2015, 6:56 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Repository: aurora Description --- DRY up PMD configuration. Diffs - build.gradle 4a13e5c4281de53d0ea991235502943a439d1310 config/pmd/custom.xml 7026d04d8e5c2f185a19c7c44d44698b2fb846c9 config/pmd/design.xml 2741e407e35826e73bd4dc8335c0ef6294086398 config/pmd/imports.xml 40dc2260a3c27fd66be352a5974427dcf165ab34 config/pmd/logging-java.xml 66fecbf14aa61f340678dc23d57a1c074cad824a config/pmd/naming.xml 233352d57857a0fe09ddbfe9228b48d4c09bb624 Diff: https://reviews.apache.org/r/32323/diff/ Testing --- ./gradlew -Pq pmdMain -d diffed the Using rule log lines from before and after: ``` % diff -u (sort -u old-rules.txt) (sort -u new-rules.txt) % ``` Thanks, Kevin Sweeney
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:15 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Changes --- Kevin rightly pointed out that i could use a real `Job()` instead of a mock, which is better on all counts. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Changes --- Updated doc. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Review Request 32319: Add a deprecation warning when using the client-side updater.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32319/ --- Review request for Aurora. Bugs: AURORA-1190 https://issues.apache.org/jira/browse/AURORA-1190 Repository: aurora Description --- See summary. Diffs - src/main/python/apache/aurora/client/cli/jobs.py 61337b9312864921508428c7f7992576b94a5d2c src/test/python/apache/aurora/client/cli/test_update.py cc7458568de121dc02de010687fcae0b2707ee0a src/test/python/apache/aurora/client/cli/util.py 864a71428f58ad5ea23beb1d9ae7520c82d2d276 Diff: https://reviews.apache.org/r/32319/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
On March 20, 2015, 10:11 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, lines 158-162 https://reviews.apache.org/r/32313/diff/4/?file=901692#file901692line158 Can we assert `self._mock_api.start_job.mock_calls = [call(...)]` instead? We can! This wasn't done before because validating the resulting `AuroraConfig` was likely deemed some combination of too complex and/or too much coupling. However, you prompted me to discover `mock.ANY`, which fits the bill here. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77284 --- On March 20, 2015, 8:23 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 8:23 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:30 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description (updated) --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:31 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
On March 20, 2015, 10:33 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, line 636 https://reviews.apache.org/r/32313/diff/5/?file=901848#file901848line636 Yes, let us add stuff! Stuff is great and junk! Stuff=added. This pointed out that i neglected to update these tests to the new style. Key result: mock.patch import is now gone! - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/#review77291 --- On March 20, 2015, 10:31 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:31 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32313: Rename 'update status' to 'update info' and support fetching arbitrary updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32313/ --- (Updated March 20, 2015, 10:48 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1206 https://issues.apache.org/jira/browse/AURORA-1206 Repository: aurora Description --- One change i snuck in here is in `cli/__init__.py`. This makes the subcommand help include the description: ``` $ aurora update info -h usage: aurora update info [-h] [--write-json] [--verbose] [--skip-hooks hook,hook,...] CLUSTER/ROLE/ENV/NAME [ID] Display detailed status information about a scheduler-driven in-progress update. If no update ID is provided, information will be displayed about the active update for the job. positional arguments: CLUSTER/ROLE/ENV/NAME Fully specified job key, in CLUSTER/ROLE/ENV/NAME format IDUpdate identifier provided by the scheduler when an update was started. optional arguments: -h, --helpshow this help message and exit --write-json Generate command output in JSON format --verbose, -v Show verbose output --skip-hooks hook,hook,... A comma-separated list of command hook names that should be skipped. If the hooks cannot be skipped, then the command will be aborted ``` Prior to this change, the description was only displayed in the parent command's help text. Diffs (updated) - docs/client-commands.md fe13fb380010f20e704608c4e82db0e5b93298ab src/main/python/apache/aurora/client/cli/__init__.py 6a0c129bc5d5dac8d8d393705a69586c9918983d src/main/python/apache/aurora/client/cli/update.py 830ef4424fe46bc8c14456492f29dea681cf5200 src/test/python/apache/aurora/client/cli/test_supdate.py f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 320c1fbeee0161528745edd38360cd1fd5d53104 Diff: https://reviews.apache.org/r/32313/diff/ Testing --- I have converted all test cases in `test_supdate.py` to use the 'new style' non-integration testing, which removed a ton of boilerplate. I also corrected some holes in the end-to-end tests, wherein `test` and conditions could silently fail. Thanks, Bill Farner
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
On March 19, 2015, 9:13 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 250 https://reviews.apache.org/r/32171/diff/4/?file=900349#file900349line250 How about a special blocked group to display only the updates blocked due to lack of heartbeat? Could be quite useful for monitoring of blocked updates. Done. On March 19, 2015, 9:13 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 258 https://reviews.apache.org/r/32171/diff/4/?file=900349#file900349line258 Move to previous line? Done. On March 19, 2015, 9:13 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 272 https://reviews.apache.org/r/32171/diff/4/?file=900349#file900349line272 Recommend adding a phrase explaining the behavior in case multiple groups/statuses are specified. Done. On March 19, 2015, 9:13 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 314 https://reviews.apache.org/r/32171/diff/4/?file=900349#file900349line314 While it's converted to set by thrift anyway, I'd still recommend using a set() here to explicitly assert no duplicates are ever accepted. Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/#review77102 --- On March 19, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 9:09 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 20, 2015, 12:29 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs (updated) - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 10:27 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs (updated) - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/#review77137 --- @ReviewBot retry I think i may have outpaced the bot here, the last reply seems for a stale diff. - Bill Farner On March 19, 2015, 10:27 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 10:27 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32225: Adding preemptor jmh benchmark
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32225/#review77143 --- Ship it! Ship It! - Bill Farner On March 20, 2015, 12:25 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32225/ --- (Updated March 20, 2015, 12:25 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Adding a preemptor slot search perf benchmark. Below are the results for synchronous (Before) and asynchronous (After) preemptor. Before: ``` Benchmark Mode Cnt ScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100781243.004 ± 9308.450 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1205278.826 ± 19800.452 ns/op SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 77048458.974 ± 918593.702 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100769919.326 ± 18963.264 ns/op ``` After: ``` Benchmark Mode CntScore Error Units SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark avgt 100 3062.264 ± 323.854 ns/op SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 10022135.031 ± 703.886 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 283028.184 ± 1954.987 ns/op SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark avgt 100 3338470.414 ± 31189.009 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 10022177.423 ± 589.332 ns/op ``` Result analysis is here: https://reviews.apache.org/r/31739/ Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java ad49effdaf700bb9d5715aa5bdd1a5d0b276f83f src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java f817ccd23644de5aa03fe42be3a5bc2b63683a9d src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java c9d10e4cec44045806ec2d75d8c158dc40d7de98 Diff: https://reviews.apache.org/r/32225/diff/ Testing --- ./gradlew jmh Thanks, Maxim Khutornenko
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/#review77140 --- @ReviewBot retry Forgive me for generating noise, i'm trying to figure out what test case is failing...not seeing this locally. I wasn't able to find the previous run in the bot's build history. - Bill Farner On March 19, 2015, 10:27 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 10:27 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32276: Fix error listing active updates.
On March 20, 2015, 1:53 a.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/update.py, line 248 https://reviews.apache.org/r/32276/diff/1/?file=901054#file901054line248 You can use frozenset here as well. It's a shame thrift sets are mutable by default (which causes them to be unhashable). Good call, done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32276/#review77153 --- On March 20, 2015, 1:53 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32276/ --- (Updated March 20, 2015, 1:53 a.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Fixes this error: ``` $ aurora update list devcluster/vagrant/test/http_example --status active Traceback (most recent call last): File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 272, in execute self.execute_entry(entry_point, args) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 320, in execute_entry runner(entry_point) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 343, in execute_pkg_resources runner() File /usr/local/bin/aurora/apache/aurora/client/cli/client.py, line 95, in proxy_main File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 329, in execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 306, in _execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 382, in execute File apache/aurora/client/cli/update.py, line 315, in execute TypeError: unhashable type: 'set' ``` Diffs - src/main/python/apache/aurora/client/cli/update.py 2168e99a315dd2916086100589c8345cd3a2c4ff Diff: https://reviews.apache.org/r/32276/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32276: Fix error listing active updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32276/ --- (Updated March 20, 2015, 2:12 a.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Fixes this error: ``` $ aurora update list devcluster/vagrant/test/http_example --status active Traceback (most recent call last): File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 272, in execute self.execute_entry(entry_point, args) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 320, in execute_entry runner(entry_point) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 343, in execute_pkg_resources runner() File /usr/local/bin/aurora/apache/aurora/client/cli/client.py, line 95, in proxy_main File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 329, in execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 306, in _execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 382, in execute File apache/aurora/client/cli/update.py, line 315, in execute TypeError: unhashable type: 'set' ``` Diffs (updated) - src/main/python/apache/aurora/client/cli/update.py 2168e99a315dd2916086100589c8345cd3a2c4ff src/test/python/apache/aurora/client/cli/test_supdate.py df072e9d9685a54c113d444b28d818b72218ffe6 Diff: https://reviews.apache.org/r/32276/diff/ Testing --- Thanks, Bill Farner
Review Request 32276: Fix error listing active updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32276/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Fixes this error: ``` $ aurora update list devcluster/vagrant/test/http_example --status active Traceback (most recent call last): File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 272, in execute self.execute_entry(entry_point, args) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 320, in execute_entry runner(entry_point) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 343, in execute_pkg_resources runner() File /usr/local/bin/aurora/apache/aurora/client/cli/client.py, line 95, in proxy_main File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 329, in execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 306, in _execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 382, in execute File /usr/local/bin/aurora/apache/aurora/client/cli/update.py, line 318, in execute TypeError: unhashable type: 'set' ``` Diffs - src/main/python/apache/aurora/client/cli/update.py 2168e99a315dd2916086100589c8345cd3a2c4ff Diff: https://reviews.apache.org/r/32276/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32276: Fix error listing active updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32276/ --- (Updated March 20, 2015, 1:53 a.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description (updated) --- Fixes this error: ``` $ aurora update list devcluster/vagrant/test/http_example --status active Traceback (most recent call last): File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 272, in execute self.execute_entry(entry_point, args) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 320, in execute_entry runner(entry_point) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 343, in execute_pkg_resources runner() File /usr/local/bin/aurora/apache/aurora/client/cli/client.py, line 95, in proxy_main File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 329, in execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 306, in _execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 382, in execute File apache/aurora/client/cli/update.py, line 315, in execute TypeError: unhashable type: 'set' ``` Diffs - src/main/python/apache/aurora/client/cli/update.py 2168e99a315dd2916086100589c8345cd3a2c4ff Diff: https://reviews.apache.org/r/32276/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 8:22 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Changes --- This diff reverts to showing the same data that was there previously, in columns instead of multiple lines. It also addresses a comment by Maxim, to allow filtering by raw status values (in addition to the shorthand values like 'active', added in this patch). Finally, i am displaying the timestamps as ISO 8601 (they were previously raw timestamp millis). Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs (updated) - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32220: Making preemptor asynchronous. Part 2 - async handling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/#review77106 --- Ship it! src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java https://reviews.apache.org/r/32220/#comment124998 This should refer to the parameter, not the guice binding annotation. src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java https://reviews.apache.org/r/32220/#comment125000 `synchronized` should be unnecessary, `Cache` is expected to be thread safe, and we're not doing multiple operations per method here. src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java https://reviews.apache.org/r/32220/#comment125001 Validates that a previously-found src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java https://reviews.apache.org/r/32220/#comment125003 Given that you probably want a singleton PreemptionSlotCache, don't forget to bind that as such here. - Bill Farner On March 19, 2015, 12:29 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/ --- (Updated March 19, 2015, 12:29 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- This diff makes preemption asynchronous wrt the scheduling loop. New flow works as follows: - TaskScheduler is unable to schedule a task and calls into PreemptorImpl.attemptPreemptionFor() to acquire a reservation. - PreemptorImpl keeps track of found preemption slots in PreemptionSlotCache, fires an async slot search request and replies back with empty result. - A search is finished and a slot (if found) is added into internal slot cache. - TaskScheduler calls into attemptPreemptionFor(), finds a preemption slot, validates a slot is still valid and preempts tasks. A reservation for a slave is created. This is still an intermediate milestone on the way to a fully independent background preemptor. Benchmark refactoring will be addressed in a separate diff. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d2cb46aa86dd4c3c6d53848725eed1542307ebd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e748b42eaa8f54e0cf1a0a883da4aceff3d7a3b8 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 1808b71546423dfe80ccb1902e8cebd545674a27 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 801a6d7f3cb0e9987e2029fd8c4c89015e8d3b65 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java dbfebf99bc6028faf433a69db4308a239ff61290 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java cfbc1a039262d92481ded2733d50ac51293a5b91 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java e329358f70028f52a807cd987378cbc002af36a9 Diff: https://reviews.apache.org/r/32220/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
On March 18, 2015, 12:03 a.m., David McLaughlin wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, line 408 https://reviews.apache.org/r/32171/diff/2/?file=898419#file898419line408 This is more of a general concern, but it would be nice to avoid adding new tests in this style - and instead test the Command instance directly. I can't find the actual back and forth that led me to testing like this, but it was introduced here: https://reviews.apache.org/r/27848/diff/# Nearly forgot to address this. Patch incoming that moves all test cases for this command to the new style. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/#review76853 --- On March 19, 2015, 8:22 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 8:22 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 9:09 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs (updated) - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32175: Add a test to ensure annotations exist for AuroraSchedulerManager
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32175/#review77113 --- Ship it! Ship It! - Bill Farner On March 17, 2015, 9:59 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32175/ --- (Updated March 17, 2015, 9:59 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- This addresses review feedback from https://reviews.apache.org/r/32141/ and prevents divergence when api.thrift changes. Diffs - src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java PRE-CREATION Diff: https://reviews.apache.org/r/32175/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 32208: Reduce loglevel for insufficient GC resources to fine
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32208/#review76947 --- Ship it! Ship It! - Bill Farner On March 18, 2015, 6:28 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32208/ --- (Updated March 18, 2015, 6:28 p.m.) Review request for Aurora. Repository: aurora Description --- Reduce loglevel for insufficient GC resources to fine Diffs - src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 Diff: https://reviews.apache.org/r/32208/diff/ Testing --- ./gradlew -Pq build Thanks, Stephan Erb
Re: Review Request 32164: Moving pending task search into PreemptorImpl
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/#review76753 --- I actually meant this should be outside the preemptor altogether. The preemptor is being called, and then internally deciding the caller should have not called in the first place. I claim this is odd behavior. I think it would make sense, instead, if the caller opened a transaction, and continued that transaction into the preemptor once it has deemed the task is `PENDING` and has been in that state long enough to warrant a preemption search. - Bill Farner On March 17, 2015, 5:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/ --- (Updated March 17, 2015, 5:19 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Addressing a TODO left after refactoring in step 1. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 9b7a3f16ad489c296d5e3513a74264a0620942a6 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e5cc48625cf019f1b6bb85749711528bf5dee4cd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java e11db4e0e04fe3ca089defc0304333cbb8bf7624 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 93cab3414e3cd76e02b34c439413e625037bf90c src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java f27ca62397e60b114f0c570cdfe9323a6c825645 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java 9a6e94dc941025fd272495de0c46f77b793e867c Diff: https://reviews.apache.org/r/32164/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32164: Moving pending task search into PreemptorImpl
On March 17, 2015, 6:05 p.m., Bill Farner wrote: I actually meant this should be outside the preemptor altogether. The preemptor is being called, and then internally deciding the caller should have not called in the first place. I claim this is odd behavior. I think it would make sense, instead, if the caller opened a transaction, and continued that transaction into the preemptor once it has deemed the task is `PENDING` and has been in that state long enough to warrant a preemption search. Maxim Khutornenko wrote: After the final refactoring step is implemented, the preemptor will be guided by a scheduled background worker most likely residing within the PreemptorImpl itself. So, it will be the only caller and consumer of pending tasks. I don't think it's worth purging pending task search out at this stage just to return it back shortly after. That makes sense, thanks for adding context. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/#review76753 --- On March 17, 2015, 5:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/ --- (Updated March 17, 2015, 5:19 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Addressing a TODO left after refactoring in step 1. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 9b7a3f16ad489c296d5e3513a74264a0620942a6 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e5cc48625cf019f1b6bb85749711528bf5dee4cd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java e11db4e0e04fe3ca089defc0304333cbb8bf7624 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 93cab3414e3cd76e02b34c439413e625037bf90c src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java f27ca62397e60b114f0c570cdfe9323a6c825645 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java 9a6e94dc941025fd272495de0c46f77b793e867c Diff: https://reviews.apache.org/r/32164/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
/workspace/AuroraBot/build-support/python/checkstyle.venv/bin Successfully installed GitPython-0.3.2rc1 gitdb-0.6.4 pep8-1.4.5 pyflakes-0.7.2 smmap-0.9.0 twitter.checkstyle-0.1.0 twitter.common.app-0.3.0 twitter.common.collections-0.3.0 twitter.common.contextutil-0.3.0 twitter.common.dirutil-0.3.0 twitter.common.lang-0.3.0 twitter.common.log-0.3.0 twitter.common.options-0.3.0 twitter.common.process-0.3.0 twitter.common.string-0.3.0 twitter.common.util-0.3.0 F841:ERROR src/main/python/apache/aurora/client/cli/update.py:316 local variable 'created' is assigned to but never used |created = summary.state.createdTimestampMs F841:ERROR src/main/python/apache/aurora/client/cli/update.py:317 local variable 'lastMod' is assigned to but never used |lastMod = summary.state.lastModifiedTimestampMs I will refresh this build result if you post a review containing @ReviewBot retry Doh sorry for the noise. Fixing. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/#review76845 --- On March 17, 2015, 11:30 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 17, 2015, 11:30 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 17, 2015, 11:30 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description (updated) --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs (updated) - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32181: Only warn about insufficient GC resources when actually needed
On March 17, 2015, 11:12 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java, line 208 https://reviews.apache.org/r/32181/diff/1/?file=898407#file898407line208 This reordering will result in pulsing the hostname even when an offer is insufficient for GC thus leading to missed collections. I'd much rather suppress the warning (e.g. change it to Level.FINE) than risk a missed collection. Stephan Erb wrote: What about extracting the pulsing from `isTimeToCollect` and performing it directly in `willUse`? Maxim Khutornenko wrote: Given this feature is going away soon, I'd rather not invest any time in improving it unless absolutely necessary. Are there any other concerns besides warnings in scheduler log? If not, perhaps lowering the logging level is the way to go. I'd be okay with LOG.fine FWIW. General +1 to removing the noise. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32181/#review76841 --- On March 17, 2015, 10:50 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32181/ --- (Updated March 17, 2015, 10:50 p.m.) Review request for Aurora. Repository: aurora Description --- Only warn about insufficient GC resources when actually needed Whenever a garbage collection run finishes, the launcher is offered the same resources again. Probably due to rounding errors, these are not considered sufficient for another run. By changing the order of time check and resource check we prevent unnecessary warnings about these small offers. Diffs - src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419 Diff: https://reviews.apache.org/r/32181/diff/ Testing --- ./gradlew -Pq build Thanks, Stephan Erb
Re: Review Request 32164: Moving pending task search into PreemptorImpl
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/#review76766 --- Ship it! Ship It! - Bill Farner On March 17, 2015, 5:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/ --- (Updated March 17, 2015, 5:19 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Addressing a TODO left after refactoring in step 1. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 9b7a3f16ad489c296d5e3513a74264a0620942a6 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e5cc48625cf019f1b6bb85749711528bf5dee4cd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java e11db4e0e04fe3ca089defc0304333cbb8bf7624 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 93cab3414e3cd76e02b34c439413e625037bf90c src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java f27ca62397e60b114f0c570cdfe9323a6c825645 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java 9a6e94dc941025fd272495de0c46f77b793e867c Diff: https://reviews.apache.org/r/32164/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31380: Now that we've identified not calling .converge as a source of flakiness, remove epsilon from health checker tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31380/#review76736 --- Is this review stale? If so, discard? - Bill Farner On Feb. 24, 2015, 9:27 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31380/ --- (Updated Feb. 24, 2015, 9:27 p.m.) Review request for Aurora and Joe Smith. Repository: aurora Description --- Now that we've identified not calling .converge as the source of the issue, remove epsilon from health checker tests. Diffs - src/test/python/apache/aurora/executor/common/test_health_checker.py 1b4423a1eb95cc950206a355fa657c8082c8d93f Diff: https://reviews.apache.org/r/31380/diff/ Testing --- ¯\_(?)_/¯ Ran this for 20 minutes on Linux and OS X: while true; do THERMOS_DEBUG=1 ./pants test.pytest --options='-v' src/test/python/apache/aurora/executor/common:health_checker; done; Thanks, Brian Wickman
Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/#review76735 --- Ship it! LGTM mod nits below and satisfying the build bot. src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java https://reviews.apache.org/r/32141/#comment124355 p for breaks. check out the rendered javadoc in intellij to see what i mean. ditto in other files src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java https://reviews.apache.org/r/32141/#comment124356 Maybe s/Annotated/Authorized/? So as to say - implementations of this interface can be assured authorization has happened. I don't feel strongly about this. - Bill Farner On March 17, 2015, 1:29 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/ --- (Updated March 17, 2015, 1:29 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to inherit from it. This gives us a place to put annotations like the AuthorizingParam one introduced in this review without needing to copy-paste them when we override a new method. A future diff will use these annotations to determine which permission a method call needs by inspecting the annotated parameter. I created a new interface to enable DRY - otherwise I'd need to annotate both ForwardingThrift and SchedulerThriftInterface and keep them in sync. Diffs - src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java f0e40b646092e96955fddc46c3a1e62a8237b00f src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java df6b53a524b005cd5fabb099fd0c08d83e3b287d src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java ee98f66de7f671018fa0a0b4894f114c7a283eda src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 3900c2228038668576cdbb37e87127246a33317c src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 7b1bf2ef8b413d2c1a08b41722a04af091305304 src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java 7e20aaa6836bd205261afe5b1244fb6af8a56356 src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java aae5cd7709abe3896c2ae06c218a0c90ca11c576 Diff: https://reviews.apache.org/r/32141/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 30818: Support separate routes for job controller tabs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/#review76737 --- Update or discard? - Bill Farner On Feb. 10, 2015, 6:01 a.m., Joshua Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/ --- (Updated Feb. 10, 2015, 6:01 a.m.) Review request for Aurora and David McLaughlin. Bugs: AURORA-696 https://issues.apache.org/jira/browse/AURORA-696 Repository: aurora Description --- Previously navigating between the active/completed/all tabs on the job controller page did not alter the browser history. Now it does, meaning that you can navigate with the back button as well as link directly to a tab. Diffs - src/main/resources/scheduler/assets/job.html 4a00a8863d676f168fbfce9f45ec4bad0244199f src/main/resources/scheduler/assets/js/app.js b66409f628046c67b62c92a75cbeed20c09b4ec1 src/main/resources/scheduler/assets/js/controllers.js 092e7d5df2121f45f99f5a788187d52bebb7e5dd Diff: https://reviews.apache.org/r/30818/diff/ Testing --- ./gradlew jsHint Verified push state worked in vagrant. Thanks, Joshua Cohen
Re: Review Request 32077: Rename beta-update subcommand to update.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32077/ --- (Updated March 17, 2015, 2:24 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1160 https://issues.apache.org/jira/browse/AURORA-1160 Repository: aurora Description --- Rename beta-update subcommand to update. Diffs - docs/client-commands.md f0769b5db58971c345090ee584fa5fc11e2a057a docs/configuration-reference.md af0386202844a0ab706c691068d587cefc75becf docs/hooks.md 23cc207700ca1cca8ae5827345a5812ab2af9d61 examples/vagrant/upstart/aurora-scheduler.conf f85127d9d565c3c91eca914f73f28005d763234c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/python/apache/aurora/client/cli/update.py cced1080655a0d6648883c16edf07f7ad75a2ca1 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 89b9ce0a4150a88e8d9099e711659b591dcdd947 src/test/python/apache/aurora/client/cli/test_supdate.py a237da92229e21253aaca488a89af2979d94ce48 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh f255f2d0a390359da4f442505fc7b8b492dc06bb Diff: https://reviews.apache.org/r/32077/diff/ Testing (updated) --- Test suite + end-to-end tests. Thanks, Bill Farner
Re: Review Request 32106: Changed the updater to not update an instance if only the job owner changes
On March 16, 2015, 4:30 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java, line 159 https://reviews.apache.org/r/32106/diff/1/?file=895820#file895820line159 There is actually a better place to do this type of action [1]. If not done there, the update will still have to iterate over all instances and generate a lot of instance event noise. Also, the UI will show these instances in the update working set, which does not reflect the NOOP nature of this action. [1] - https://github.com/apache/incubator-aurora/blob/master/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java Bill Farner wrote: Good point. Steve landed at this line based on my hasty direction. You're right, though, that JobDiff is the place to move it. Perhaps this comparison should be done in JobDiff as well, to prevent impedance mismatch? Also, better yet - a test case in `JobUpdaterIT` should have pointed out this flaw. Steve - can you add a case there? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32106/#review76568 --- On March 16, 2015, 3:13 p.m., Steve Niemitz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32106/ --- (Updated March 16, 2015, 3:13 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1200 https://issues.apache.org/jira/browse/AURORA-1200 Repository: aurora Description --- Changed the updater to not update an instance if the job owner changes Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 14753cf5ef35d5133ca5029f3a465df884756070 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 09c147e76d7c2c130a1fdd85459c45395fee7dde Diff: https://reviews.apache.org/r/32106/diff/ Testing --- Thanks, Steve Niemitz
Re: Review Request 32077: Rename beta-update subcommand to update.
On March 16, 2015, 6:22 p.m., Zameer Manji wrote: Pending e2e test results. FYI end-to-end tests pass with this change, _in a fresh vagrant environment_. Existing envorinments will be broken, see details and proposed improvement in AURORA-1204. I will e-mail the dev@ list after committing to let folks know that they need to destroy/create. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32077/#review76599 --- On March 16, 2015, 5:57 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32077/ --- (Updated March 16, 2015, 5:57 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1160 https://issues.apache.org/jira/browse/AURORA-1160 Repository: aurora Description --- Rename beta-update subcommand to update. Diffs - docs/client-commands.md f0769b5db58971c345090ee584fa5fc11e2a057a docs/configuration-reference.md af0386202844a0ab706c691068d587cefc75becf docs/hooks.md 23cc207700ca1cca8ae5827345a5812ab2af9d61 examples/vagrant/upstart/aurora-scheduler.conf f85127d9d565c3c91eca914f73f28005d763234c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/python/apache/aurora/client/cli/update.py cced1080655a0d6648883c16edf07f7ad75a2ca1 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 89b9ce0a4150a88e8d9099e711659b591dcdd947 src/test/python/apache/aurora/client/cli/test_supdate.py a237da92229e21253aaca488a89af2979d94ce48 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh f255f2d0a390359da4f442505fc7b8b492dc06bb Diff: https://reviews.apache.org/r/32077/diff/ Testing --- Running end-to-end tests in an existing vagrant setup, and will repeat with a fresh one. Will report back before committing. Thanks, Bill Farner
Review Request 32078: Remove the populatedDEPRECATED thrift field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32078/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-975 https://issues.apache.org/jira/browse/AURORA-975 Repository: aurora Description --- Remove the populatedDEPRECATED thrift field. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 8386ee36117c8135c7f855a496e1e887904b23dd src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 3876290c3c116ae29a0f386c899ea213efdba318 src/main/python/apache/aurora/client/api/updater.py 7fd6fdacafb7c3d2b30e1d9de2c8a48a88e0e943 src/main/python/apache/aurora/client/base.py 352d3f07148eff3c616f1f2dba861de6a023acc7 src/main/python/apache/aurora/client/cli/jobs.py d6633d93a09f5203219d680cf3780ba1f17d0969 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 3cf8dfde64d38d43e72dd1e7ccc901584b59cbe9 src/test/python/apache/aurora/client/api/test_updater.py 7f3bc28aef52ed171a24ce2a98718afb6bfb1b54 src/test/python/apache/aurora/client/cli/test_diff.py 054e6fbc1b1a56b4818b2a8f08b06e8c64bcfc10 src/test/python/apache/aurora/client/cli/test_restart.py 4f20f6bf3db8d9cae35ec6458777990c763cdbd1 src/test/python/apache/aurora/client/cli/test_update.py 9bfbbea85c6f123076fb570ea91d154dd11c2d78 src/test/python/apache/aurora/client/test_base.py 06c0b436b5104c9b3afbf80499d3bfc66e7ef2f4 Diff: https://reviews.apache.org/r/32078/diff/ Testing --- Thanks, Bill Farner
Review Request 32077: Rename beta-update subcommand to update.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32077/ --- Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1160 https://issues.apache.org/jira/browse/AURORA-1160 Repository: aurora Description --- Rename beta-update subcommand to update. Diffs - docs/client-commands.md f0769b5db58971c345090ee584fa5fc11e2a057a docs/configuration-reference.md af0386202844a0ab706c691068d587cefc75becf docs/hooks.md 23cc207700ca1cca8ae5827345a5812ab2af9d61 examples/vagrant/upstart/aurora-scheduler.conf f85127d9d565c3c91eca914f73f28005d763234c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/python/apache/aurora/client/cli/update.py cced1080655a0d6648883c16edf07f7ad75a2ca1 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 89b9ce0a4150a88e8d9099e711659b591dcdd947 src/test/python/apache/aurora/client/cli/test_supdate.py a237da92229e21253aaca488a89af2979d94ce48 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh f255f2d0a390359da4f442505fc7b8b492dc06bb Diff: https://reviews.apache.org/r/32077/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32106: Changed the updater to not update an instance if only the job owner changes
On March 16, 2015, 4:30 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java, line 159 https://reviews.apache.org/r/32106/diff/1/?file=895820#file895820line159 There is actually a better place to do this type of action [1]. If not done there, the update will still have to iterate over all instances and generate a lot of instance event noise. Also, the UI will show these instances in the update working set, which does not reflect the NOOP nature of this action. [1] - https://github.com/apache/incubator-aurora/blob/master/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java Good point. Steve landed at this line based on my hasty direction. You're right, though, that JobDiff is the place to move it. Perhaps this comparison should be done in JobDiff as well, to prevent impedance mismatch? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32106/#review76568 --- On March 16, 2015, 3:13 p.m., Steve Niemitz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32106/ --- (Updated March 16, 2015, 3:13 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1200 https://issues.apache.org/jira/browse/AURORA-1200 Repository: aurora Description --- Changed the updater to not update an instance if the job owner changes Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 14753cf5ef35d5133ca5029f3a465df884756070 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 09c147e76d7c2c130a1fdd85459c45395fee7dde Diff: https://reviews.apache.org/r/32106/diff/ Testing --- Thanks, Steve Niemitz
Re: Review Request 32077: Rename beta-update subcommand to update.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32077/ --- (Updated March 16, 2015, 5:57 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1160 https://issues.apache.org/jira/browse/AURORA-1160 Repository: aurora Description --- Rename beta-update subcommand to update. Diffs - docs/client-commands.md f0769b5db58971c345090ee584fa5fc11e2a057a docs/configuration-reference.md af0386202844a0ab706c691068d587cefc75becf docs/hooks.md 23cc207700ca1cca8ae5827345a5812ab2af9d61 examples/vagrant/upstart/aurora-scheduler.conf f85127d9d565c3c91eca914f73f28005d763234c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/python/apache/aurora/client/cli/update.py cced1080655a0d6648883c16edf07f7ad75a2ca1 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 89b9ce0a4150a88e8d9099e711659b591dcdd947 src/test/python/apache/aurora/client/cli/test_supdate.py a237da92229e21253aaca488a89af2979d94ce48 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh f255f2d0a390359da4f442505fc7b8b492dc06bb Diff: https://reviews.apache.org/r/32077/diff/ Testing (updated) --- Running end-to-end tests in an existing vagrant setup, and will repeat with a fresh one. Will report back before committing. Thanks, Bill Farner
Re: Review Request 32118: Move creation of auth_module into ThriftAuthModule.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32118/#review76613 --- Ship it! Ship It! - Bill Farner On March 16, 2015, 6:54 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32118/ --- (Updated March 16, 2015, 6:54 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1201 https://issues.apache.org/jira/browse/AURORA-1201 Repository: aurora Description --- Move creation of auth_module into ThriftAuthModule. Diffs - src/main/java/org/apache/aurora/scheduler/app/Modules.java e86e35f5435e7e83956fd80c8d3ae39eb50c9170 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java c6f8787c894533327114425ba17bca966f07b554 src/main/java/org/apache/aurora/scheduler/thrift/auth/ThriftAuthModule.java e390af12669ac9464e2e7f6e9e1288136ed5ed9b src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java bb19fc72cafc1640f6331c5eaa3761da1a4af7c0 Diff: https://reviews.apache.org/r/32118/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Review Request 32050: Show update status change messages in the scheduler UI.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32050/ --- Review request for Aurora and David McLaughlin. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Show update status change messages in the scheduler UI. Diffs - src/main/resources/scheduler/assets/update.html 2ce13b4cafaa3c219ff1ee60231f954b398f193d src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 5cff81f4d8d220fb6c18005884d2f7a8beabaa37 Diff: https://reviews.apache.org/r/32050/diff/ Testing --- Ran end-to-end tests, confirmed pause message is present in the job state transitions table. Thanks, Bill Farner
Re: Review Request 32014: Adding more logging into MaintenanceController.
On March 13, 2015, 2:30 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java, line 279 https://reviews.apache.org/r/32014/diff/1/?file=892576#file892576line279 Ditto - this really seems like it should be client-side logging. The client gets back the actiosn that were taken, seems like it has all the info it needs to check for discrepancies. Maxim Khutornenko wrote: This is particularly important for troubleshooting as we have occasionally observed abnormal behavior that possibly points to host attributes handling. Lack of logs around that does not help us there. Given the relatively infrequent use of this functionality, I'd rather have more data points here. Aha, i was operating on the incorrect assumption that this plumbed directly out to the thift interface, but it has internal callers as well. On March 13, 2015, 2:30 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java, line 255 https://reviews.apache.org/r/32014/diff/1/?file=892576#file892576line255 This is a pretty weird log entry to have. Can it be done in the client instead? Maxim Khutornenko wrote: Client already has logs but that's not enough to reconstruct the behavior when maintenance stalls. We need to see both sides of the story to properly troubleshoot host draining issues. This one i still don't follow - AFAICT it's literally printing the response handed directly to the client, which is the only caller if i'm reading correctly. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/#review76332 --- On March 13, 2015, 1:23 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- (Updated March 13, 2015, 1:23 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Also added missing test coverage. Diffs - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 32051: Detect an invalid .auroraversion expansion and provide a helpful error message.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32051/ --- Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1169 https://issues.apache.org/jira/browse/AURORA-1169 Repository: aurora Description --- Help users notice and make progress more quickly when fatefully attempting to build with a Windows host OS. An alternative approach would be to not rely on the symlink here, but i'm not confident that this would kick the can down the road to one of the numerous other places we rely on symlinks. This does not fix AURORA-1169, but i propose that we do this to make it clear earlier that Windows as a build platform is not currently supported. Given the number of native dependnecies we have, i think this is the most sane course for the project today. Diffs - build.gradle dc366d4339b83af9e1fea663fe8e5860702644e9 Diff: https://reviews.apache.org/r/32051/diff/ Testing --- Ran the build with a dummy version value, got this: ``` $ ./gradlew build :buildSrc:compileJava UP-TO-DATE :buildSrc:compileGroovy UP-TO-DATE :buildSrc:processResources UP-TO-DATE :buildSrc:classes UP-TO-DATE :buildSrc:jar UP-TO-DATE :buildSrc:assemble UP-TO-DATE :buildSrc:compileTestJava UP-TO-DATE :buildSrc:compileTestGroovy UP-TO-DATE :buildSrc:processTestResources UP-TO-DATE :buildSrc:testClasses UP-TO-DATE :buildSrc:test UP-TO-DATE :buildSrc:check UP-TO-DATE :buildSrc:build UP-TO-DATE FAILURE: Build failed with an exception. * Where: Build file '/home/wfarner/code/aurora/build.gradle' line: 53 * What went wrong: A problem occurred evaluating root project 'aurora'. *** The application version string read by gradle is invalid. This is known to happen when building on Windows, or within vagrant with a Windows host. You can work around this issue by cloning git _within_ vagrant and building from there. For more details, please see https://issues.apache.org/jira/browse/AURORA-1169 *** * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 6.14 secs ``` Thanks, Bill Farner
Re: Review Request 31966: Add client support for including messages when changing update state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/ --- (Updated March 13, 2015, 1:58 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- More plumbing. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 4025781963a61821d111e6347ae02d27459fd8cd src/main/python/apache/aurora/client/cli/update.py 37cc49880eab2ac0879945714634a4938b603aae src/main/python/apache/aurora/client/hooks/hooked_api.py 60a5aad7c3a7388154654673a6669c93be716635 src/test/python/apache/aurora/api_util.py 70599555c8b13073e51adee31662df74afe67edb src/test/python/apache/aurora/client/api/test_api.py d211fb975db01e72a88312c28078e06cb622d83c src/test/python/apache/aurora/client/cli/test_supdate.py 370a46b46d8fe9499468e0792f181e79e8042b61 Diff: https://reviews.apache.org/r/31966/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32014: Adding more logging into MaintenanceController.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/#review76332 --- src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java https://reviews.apache.org/r/32014/#comment123873 `Tasks.ids(activeTasks)` src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java https://reviews.apache.org/r/32014/#comment123874 This is a pretty weird log entry to have. Can it be done in the client instead? src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java https://reviews.apache.org/r/32014/#comment123875 Ditto - this really seems like it should be client-side logging. The client gets back the actiosn that were taken, seems like it has all the info it needs to check for discrepancies. - Bill Farner On March 13, 2015, 1:23 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- (Updated March 13, 2015, 1:23 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Also added missing test coverage. Diffs - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31966: Add client support for including messages when changing update state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/ --- (Updated March 13, 2015, 2:12 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Changes --- Neglected to commit hooked_api.py change. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- More plumbing. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 4025781963a61821d111e6347ae02d27459fd8cd src/main/python/apache/aurora/client/cli/update.py 37cc49880eab2ac0879945714634a4938b603aae src/main/python/apache/aurora/client/hooks/hooked_api.py 60a5aad7c3a7388154654673a6669c93be716635 src/test/python/apache/aurora/api_util.py 70599555c8b13073e51adee31662df74afe67edb src/test/python/apache/aurora/client/api/test_api.py d211fb975db01e72a88312c28078e06cb622d83c src/test/python/apache/aurora/client/cli/test_supdate.py 370a46b46d8fe9499468e0792f181e79e8042b61 Diff: https://reviews.apache.org/r/31966/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31966: Add client support for including messages when changing update state.
On March 12, 2015, 4:23 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/api/__init__.py, line 157 https://reviews.apache.org/r/31966/diff/2/?file=891871#file891871line157 Spacing seems off here and below. Fixed. On March 12, 2015, 4:23 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 104 https://reviews.apache.org/r/31966/diff/2/?file=891872#file891872line104 I'd also include a short '-m' name to make typing less tedious. Done. On March 12, 2015, 4:23 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 107 https://reviews.apache.org/r/31966/diff/2/?file=891872#file891872line107 While not entirely consistent, the majority of our CommandOption help strings start with a capital. Changed. On March 12, 2015, 4:23 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, lines 123-127 https://reviews.apache.org/r/31966/diff/2/?file=891872#file891872line123 Mind sorting them alphabetically? Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/#review76243 --- On March 12, 2015, 1:27 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/ --- (Updated March 12, 2015, 1:27 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- More plumbing. Diffs - src/main/python/apache/aurora/client/api/__init__.py 4025781963a61821d111e6347ae02d27459fd8cd src/main/python/apache/aurora/client/cli/update.py 37cc49880eab2ac0879945714634a4938b603aae src/main/python/apache/aurora/client/hooks/hooked_api.py 60a5aad7c3a7388154654673a6669c93be716635 src/test/python/apache/aurora/api_util.py 70599555c8b13073e51adee31662df74afe67edb src/test/python/apache/aurora/client/api/test_api.py d211fb975db01e72a88312c28078e06cb622d83c src/test/python/apache/aurora/client/cli/test_supdate.py 370a46b46d8fe9499468e0792f181e79e8042b61 Diff: https://reviews.apache.org/r/31966/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31966: Add client support for including messages when changing update state.
On March 12, 2015, 4:23 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, lines 123-127 https://reviews.apache.org/r/31966/diff/2/?file=891872#file891872line123 Mind sorting them alphabetically? Bill Farner wrote: Done. Spoke too soon here - ordering is important with these, and this pointed out that my new field should have been named `MESSAGE_OPTION` (as it's an option rather than an argument). I've sorted the options, which are order-agnostic. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/#review76243 --- On March 12, 2015, 1:27 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/ --- (Updated March 12, 2015, 1:27 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- More plumbing. Diffs - src/main/python/apache/aurora/client/api/__init__.py 4025781963a61821d111e6347ae02d27459fd8cd src/main/python/apache/aurora/client/cli/update.py 37cc49880eab2ac0879945714634a4938b603aae src/main/python/apache/aurora/client/hooks/hooked_api.py 60a5aad7c3a7388154654673a6669c93be716635 src/test/python/apache/aurora/api_util.py 70599555c8b13073e51adee31662df74afe67edb src/test/python/apache/aurora/client/api/test_api.py d211fb975db01e72a88312c28078e06cb622d83c src/test/python/apache/aurora/client/cli/test_supdate.py 370a46b46d8fe9499468e0792f181e79e8042b61 Diff: https://reviews.apache.org/r/31966/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32012: Improve TaskDetector performance by memoizing regular expressions
On March 13, 2015, 1:10 a.m., Brian Wickman wrote: @ReviewBot retry If you believe you have discovered a flaky test (which i assume you're asking the bot to try again), can you please file a ticket? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32012/#review76322 --- On March 13, 2015, 12:47 a.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32012/ --- (Updated March 13, 2015, 12:47 a.m.) Review request for Aurora, Joe Smith and Zameer Manji. Repository: aurora Description --- We found that the new observer burns 10x the cpu in production. Most of the time is spent in compiling new regular expressions, so memoize them. Follow-up review adds TaskDetector caching in observer. Diffs - src/main/python/apache/thermos/common/path.py 4359a5143551fff3cf66e39e68ba9ba02b8d6e2e src/main/python/apache/thermos/monitoring/detector.py e0922e61ef03fc578988f992aba46570ea1b5d6c src/test/python/apache/thermos/monitoring/BUILD 4e99da403f322940579fd4f9412e022df0790884 src/test/python/apache/thermos/monitoring/test_detector.py e9005c402ec81bbb415d9c814aa7b4e759138304 Diff: https://reviews.apache.org/r/32012/diff/ Testing --- Added generic TaskDetector test and made sure it passes before/after. Thanks, Brian Wickman
Re: Review Request 31828: Upgrade gradle and a few plugins.
On March 9, 2015, 5:58 p.m., Kevin Sweeney wrote: LGTM once reviewbot is happy. It might be choking on the binary diff. Yeah, pretty sure that's the case :-( - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31828/#review75729 --- On March 7, 2015, 5:54 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31828/ --- (Updated March 7, 2015, 5:54 p.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Gradle 2.3: http://gradle.org/docs/2.3/release-notes Checkstyle: http://checkstyle.sourceforge.net/releasenotes.html Can't tell what, if anything, changed in the versions plugin: https://github.com/ben-manes/gradle-versions-plugin/releases Diffs - build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 buildSrc/gradle.properties 73341fa90f20d73edf34fc3c945b686dd14bf941 gradle/wrapper/gradle-wrapper.jar 2322723c7ed5f591adfa4c87b6c51932f591249a gradle/wrapper/gradle-wrapper.properties 531b41ced8b99d418636a18f4f97297722944ea3 Diff: https://reviews.apache.org/r/31828/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
On March 11, 2015, 12:19 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line 48 https://reviews.apache.org/r/31916/diff/1/?file=890852#file890852line48 Do we really want to fail an operation when a message gets too long? Since it's optional anyway, I'd expect truncation could be a more resilient way to handle this. David McLaughlin wrote: I think it's better to just give a clear message telling them there is a limit. Truncation could happen in the client if needed. Maxim Khutornenko wrote: I was considering a case when some automated external service would attempt to act on an update and append some arbitrary metadata with pause/resume/abort. While not desirable, does not necessarily warrant a failure. Stopping a misbehaving update should clearly be a higher priority than enforcing a strict audit mode. Bill Farner wrote: Maybe it does warrant a failure, though. IMHO truncation would be a policy decision that the scheduler is making on behalf of the client. If the most important part of the message is after the truncation, we've made a poor choice. Maxim Khutornenko wrote: IDK, this whole length enforcment seems quite arbitrary to me. We don't restrict string size anywhere in thrift API or SQL schema. The only exception is TaskIdGenerator where the ID length is critical for external reasons (MESOS-691). Perhaps we should start doing it everywhere then? Joshua Cohen wrote: If the requirement to restrict message length is not driven by the underlying store, is there any reason to enforce it at all? Could we just let clients truncate messages as appropriate to their use case? Are we concerned with abuse and/or performance impact? this whole length enforcment seems quite arbitrary to me I find this surprising. To me it seems reasonable to be defensive and protect a critical service from arbitary-length inputs. We don't restrict string size anywhere in thrift API or SQL schema We do in some places, and we used to in more. IMHO ideally we would do this in every place we have arbitrary input. If the requirement to restrict message length is not driven by the underlying store, is there any reason to enforce it at all? IMHO if a user can crash our service, we have failed. (There are obviously tradeoffs here.) In this case, i could cripple Aurora using this input. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76008 --- On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
On March 11, 2015, 12:19 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line 48 https://reviews.apache.org/r/31916/diff/1/?file=890852#file890852line48 Do we really want to fail an operation when a message gets too long? Since it's optional anyway, I'd expect truncation could be a more resilient way to handle this. David McLaughlin wrote: I think it's better to just give a clear message telling them there is a limit. Truncation could happen in the client if needed. Maxim Khutornenko wrote: I was considering a case when some automated external service would attempt to act on an update and append some arbitrary metadata with pause/resume/abort. While not desirable, does not necessarily warrant a failure. Stopping a misbehaving update should clearly be a higher priority than enforcing a strict audit mode. Bill Farner wrote: Maybe it does warrant a failure, though. IMHO truncation would be a policy decision that the scheduler is making on behalf of the client. If the most important part of the message is after the truncation, we've made a poor choice. Maxim Khutornenko wrote: IDK, this whole length enforcment seems quite arbitrary to me. We don't restrict string size anywhere in thrift API or SQL schema. The only exception is TaskIdGenerator where the ID length is critical for external reasons (MESOS-691). Perhaps we should start doing it everywhere then? Joshua Cohen wrote: If the requirement to restrict message length is not driven by the underlying store, is there any reason to enforce it at all? Could we just let clients truncate messages as appropriate to their use case? Are we concerned with abuse and/or performance impact? Bill Farner wrote: this whole length enforcment seems quite arbitrary to me I find this surprising. To me it seems reasonable to be defensive and protect a critical service from arbitary-length inputs. We don't restrict string size anywhere in thrift API or SQL schema We do in some places, and we used to in more. IMHO ideally we would do this in every place we have arbitrary input. If the requirement to restrict message length is not driven by the underlying store, is there any reason to enforce it at all? IMHO if a user can crash our service, we have failed. (There are obviously tradeoffs here.) In this case, i could cripple Aurora using this input. Circling back from offline discussion between myself, Maxim, and Joshua - we should aim to constrain these values at the edge of the system (thrift API) to provide useful error messages to users, and do so additionally in the database. I will proceed with this change as-is and follow up with an input validation layer in https://issues.apache.org/jira/browse/AURORA-1188 - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76008 --- On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Review Request 31966: Add client support for including messages when changing update state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/ --- Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- More plumbing. Diffs - src/main/python/apache/aurora/client/api/__init__.py 4025781963a61821d111e6347ae02d27459fd8cd src/main/python/apache/aurora/client/cli/update.py 37cc49880eab2ac0879945714634a4938b603aae src/main/python/apache/aurora/client/hooks/hooked_api.py 60a5aad7c3a7388154654673a6669c93be716635 src/test/python/apache/aurora/api_util.py 70599555c8b13073e51adee31662df74afe67edb src/test/python/apache/aurora/client/api/test_api.py d211fb975db01e72a88312c28078e06cb622d83c src/test/python/apache/aurora/client/cli/test_supdate.py 370a46b46d8fe9499468e0792f181e79e8042b61 Diff: https://reviews.apache.org/r/31966/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31966: Add client support for including messages when changing update state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/#review76193 --- @ReviewBot retry - Bill Farner On March 12, 2015, 1:27 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/ --- (Updated March 12, 2015, 1:27 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- More plumbing. Diffs - src/main/python/apache/aurora/client/api/__init__.py 4025781963a61821d111e6347ae02d27459fd8cd src/main/python/apache/aurora/client/cli/update.py 37cc49880eab2ac0879945714634a4938b603aae src/main/python/apache/aurora/client/hooks/hooked_api.py 60a5aad7c3a7388154654673a6669c93be716635 src/test/python/apache/aurora/api_util.py 70599555c8b13073e51adee31662df74afe67edb src/test/python/apache/aurora/client/api/test_api.py d211fb975db01e72a88312c28078e06cb622d83c src/test/python/apache/aurora/client/cli/test_supdate.py 370a46b46d8fe9499468e0792f181e79e8042b61 Diff: https://reviews.apache.org/r/31966/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31966: Add client support for including messages when changing update state.
On March 12, 2015, 1:38 a.m., Aurora ReviewBot wrote:
Master (f62e0f1) is red with this patch.
./build-support/jenkins/build.sh
01:37:22 00:00 [deferred-sources]
01:37:22 00:00 [deferred-sources]
01:37:22 00:00 [gen]
01:37:22 00:00 [thrift][32m
Invalidated 4 targets containing 4 payload
files.[0m[32m
Generating thrift for
api/src/main/thrift/org/apache/aurora/gen/test.thrift
[0m[32m
Generating thrift for
api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift
[0m[32m
Generating thrift for
api/src/main/thrift/org/apache/thermos/thermos_internal.thrift
[0m[32m
Generating thrift for
api/src/main/thrift/org/apache/aurora/gen/storage.thrift
[0m
01:37:22 00:00 [scrooge]
01:37:22 00:00 [protoc]
01:37:22 00:00 [antlr]
01:37:22 00:00 [ragel]
01:37:22 00:00 [jaxb]
01:37:23 00:01 [wire]
01:37:23 00:01 [aapt]
01:37:23 00:01 [resolve]
01:37:23 00:01 [ivy]
01:37:23 00:01 [bootstrap-nailgun-server]
01:37:23 00:01 [compile]
01:37:23 00:01 [compile]
01:37:23 00:01 [jvm]
01:37:23 00:01 [jvm-compilers]
01:37:23 00:01 [resources]
01:37:23 00:01 [prepare][32m
Invalidated 1 target containing 1 payload file.[0m
01:37:23 00:01 [test]
01:37:23 00:01 [run_prep_command]
01:37:23 00:01 [test]
01:37:23 00:01 [pytest]
01:37:25 00:03 [run]
[31m
FAILURE[0m
Exception message: Cannot satisfy requirements:
[Requirement.parse('py=1.4.25')]
I will refresh this build result if you post a review containing
@ReviewBot retry
fun
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31966/#review76191
---
On March 12, 2015, 1:27 a.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31966/
---
(Updated March 12, 2015, 1:27 a.m.)
Review request for Aurora, Maxim Khutornenko and Zameer Manji.
Bugs: AURORA-1077
https://issues.apache.org/jira/browse/AURORA-1077
Repository: aurora
Description
---
More plumbing.
Diffs
-
src/main/python/apache/aurora/client/api/__init__.py
4025781963a61821d111e6347ae02d27459fd8cd
src/main/python/apache/aurora/client/cli/update.py
37cc49880eab2ac0879945714634a4938b603aae
src/main/python/apache/aurora/client/hooks/hooked_api.py
60a5aad7c3a7388154654673a6669c93be716635
src/test/python/apache/aurora/api_util.py
70599555c8b13073e51adee31662df74afe67edb
src/test/python/apache/aurora/client/api/test_api.py
d211fb975db01e72a88312c28078e06cb622d83c
src/test/python/apache/aurora/client/cli/test_supdate.py
370a46b46d8fe9499468e0792f181e79e8042b61
Diff: https://reviews.apache.org/r/31966/diff/
Testing
---
Thanks,
Bill Farner
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
On March 11, 2015, 12:19 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line 48 https://reviews.apache.org/r/31916/diff/1/?file=890852#file890852line48 Do we really want to fail an operation when a message gets too long? Since it's optional anyway, I'd expect truncation could be a more resilient way to handle this. David McLaughlin wrote: I think it's better to just give a clear message telling them there is a limit. Truncation could happen in the client if needed. Maxim Khutornenko wrote: I was considering a case when some automated external service would attempt to act on an update and append some arbitrary metadata with pause/resume/abort. While not desirable, does not necessarily warrant a failure. Stopping a misbehaving update should clearly be a higher priority than enforcing a strict audit mode. Maybe it does warrant a failure, though. IMHO truncation would be a policy decision that the scheduler is making on behalf of the client. If the most important part of the message is after the truncation, we've made a poor choice. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76008 --- On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31869: Catch only known Exception types in the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 10, 2015, 5:44 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31901: Export task status reason counters whenever they are present.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31901/ --- (Updated March 10, 2015, 6 p.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- I realized that the reason values apply to more than just TASK_LOST, so the previous code would hide reasons like when a task has exceeded its memory limit. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java 68214f20dd7f46adec2d8f6d84e9840dc88dc0fb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java 4bbeff957050fb9d8ee81d9fc79520a6a0ac38a1 Diff: https://reviews.apache.org/r/31901/diff/ Testing --- Thanks, Bill Farner
Review Request 31901: Export task status reason counters whenever they are present.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31901/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- I realized that the reason values apply to more than just TASK_LOST, so the previous code would hide reasons like when a task has exceeded its memory limit. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java 68214f20dd7f46adec2d8f6d84e9840dc88dc0fb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java 4bbeff957050fb9d8ee81d9fc79520a6a0ac38a1 Diff: https://reviews.apache.org/r/31901/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/#review75954 --- Ship it! Ship It! - Bill Farner On March 10, 2015, 7:45 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/ --- (Updated March 10, 2015, 7:45 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-809 and AURORA-811 https://issues.apache.org/jira/browse/AURORA-809 https://issues.apache.org/jira/browse/AURORA-811 Repository: aurora Description --- * Add dependency on Apache Shiro. * HTTP Basic Authentication. * Authorization based on shiro.ini. * Sample shiro.ini for local mode. Diffs - build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 24b61c1e4f615295acf28d904588e1512972d3f4 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 8a59d89c07b406ce98076ca7ee51b958599a39ec src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParser.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 1e4ba014804b56a2ea02770d09beb63faaabf684 src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 640acdf4e73f99418473ca97bcdc4f5f4c190f10 src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroIniParserTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 52fe0ea063dbc7a71a20926630bf449dbd936306 src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-example.ini PRE-CREATION src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-extra-sections.ini PRE-CREATION src/test/resources/org/apache/aurora/scheduler/http/api/security/shiro-malformed-missing-sections.ini PRE-CREATION Diff: https://reviews.apache.org/r/31820/diff/ Testing --- ./gradlew -Pq build Local testing in the UI and with cURL. Updates to e2e test and Vagrant environment to follow. Thanks, Kevin Sweeney
Re: Review Request 31869: Catch only known Exception types in the client.
On March 11, 2015, 12:08 a.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/__init__.py, line 282 https://reviews.apache.org/r/31869/diff/3/?file=890382#file890382line282 My understanding was that zmanji was attempting to remove AuroraCommandContext and patch print instead. Perhaps you and him should come to a consensus. Did he not express agreement by giving a ship-it on this diff? On March 11, 2015, 12:08 a.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/api/__init__.py, line 134 https://reviews.apache.org/r/31869/diff/3/?file=890381#file890381line134 Revert this - self will correctly find the class field. Done. On March 11, 2015, 12:08 a.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 681 https://reviews.apache.org/r/31869/diff/3/?file=890383#file890383line681 revert to use self so that subclass delegation will work. Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/#review75980 --- On March 10, 2015, 5:44 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 10, 2015, 5:44 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31869: Catch only known Exception types in the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 11, 2015, 12:17 a.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31869: Catch only known Exception types in the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 11, 2015, 12:26 a.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31862: Fix preformatted text when viewed on github
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31862/#review75728 --- Ship it! Thanks! This is now on master: ``` $ git log -1 origin/master commit 57b5b15ab55ea4cfe32e8ed0a60a219331df1d75 Author: Brian Brazil brian.bra...@boxever.com Date: Mon Mar 9 10:52:29 2015 -0700 Fix preformatted text when viewed on github Reviewed at https://reviews.apache.org/r/31862/ ``` - Bill Farner On March 9, 2015, 5:47 p.m., Brian Brazil wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31862/ --- (Updated March 9, 2015, 5:47 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Fix preformatted text when viewed on github Diffs - README.md 6f2731b920fa06a30c557d823ef43d5693759e93 Diff: https://reviews.apache.org/r/31862/diff/ Testing --- Fixed version: https://github.com/brian-brazil/incubator-aurora/blob/readme/README.md Thanks, Brian Brazil
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 9, 2015, 5:50 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 291 https://reviews.apache.org/r/31814/diff/2/?file=888120#file888120line291 What's the motivation behind dropping the Aurora Updater here? Bill Farner wrote: I think the magic user value abuses a field that serves a different purpose. IMHO an API consumer should be able to programmatically determine that the scheduler independently performed an action without resorting to string matching on the magic value of a field. In this particular case, i don't think the user field adds signal to what is already present in the state enum value. Maxim Khutornenko wrote: I still think having a special Aurora Updater user clearly visible in the UI makes it way easier to grasp the event origin rather than trying to decipher a particular state meaning. Users may not and should not be familar with the update state diagram in order to understand the cause of the transition. This is especially true in a multi-actor environment where state transitions may come from users (pause/resume/abort), external monitoring service (pause) or the updater itself (pulse expired). Bill Farner wrote: I still think having a special Aurora Updater user clearly visible in the UI I completely agree, but the UI should make that decision, not the scheduler. Maxim Khutornenko wrote: I see, sure I am fine with that. Do you propose to address it separately? If so, mind filing a ticket? Kevin Sweeney wrote: +1 to removal of the magic string - hacking the ui should be done in the ui. Went with a more broad ticket to cover this: https://issues.apache.org/jira/browse/AURORA-1178 - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75726 --- On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31820: Support HTTP Basic auth and shiro.ini configuration
On March 9, 2015, 6:36 p.m., Zameer Manji wrote: src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java, line 50 https://reviews.apache.org/r/31820/diff/1/?file=888081#file888081line50 Would it be possible to create a Ini parser for Args instead of just holding the path? +1, this would make for more natural error output in some trivial misconfiguration cases. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/#review75736 --- On March 7, 2015, 12:54 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31820/ --- (Updated March 7, 2015, 12:54 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-809 and AURORA-811 https://issues.apache.org/jira/browse/AURORA-809 https://issues.apache.org/jira/browse/AURORA-811 Repository: aurora Description --- * Add dependency on Apache Shiro. * HTTP Basic Authentication. * Authorization based on shiro.ini. * Sample shiro.ini for local mode. Diffs - build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 24b61c1e4f615295acf28d904588e1512972d3f4 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 8a59d89c07b406ce98076ca7ee51b958599a39ec src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 3e7483b1e4e674397fd093f1e301d9cb2d3ca166 src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 1e4ba014804b56a2ea02770d09beb63faaabf684 src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 640acdf4e73f99418473ca97bcdc4f5f4c190f10 src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 52fe0ea063dbc7a71a20926630bf449dbd936306 src/test/resources/org/apache/aurora/scheduler/app/local/shiro.ini PRE-CREATION Diff: https://reviews.apache.org/r/31820/diff/ Testing --- ./gradlew -Pq build Local testing in the UI and with cURL. Updates to e2e test and Vagrant environment to follow. Thanks, Kevin Sweeney
Re: Review Request 31869: Catch only known Exception types in the client.
On March 9, 2015, 8:17 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 679 https://reviews.apache.org/r/31869/diff/1/?file=889600#file889600line679 why not just context? Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/#review75761 --- On March 9, 2015, 8:10 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 9, 2015, 8:10 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31869: Catch only known Exception types in the client.
On March 9, 2015, 8:21 p.m., Zameer Manji wrote: src/test/python/apache/aurora/client/cli/test_create.py, line 277 https://reviews.apache.org/r/31869/diff/1/?file=889602#file889602line277 Use pytest.raises for tests where you expect an exception to be raised: http://pytest.org/latest/assert.html Thanks, i should have looked harder for the convention here. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/#review75760 --- On March 9, 2015, 8:10 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 9, 2015, 8:10 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 9, 2015, 5:50 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 291 https://reviews.apache.org/r/31814/diff/2/?file=888120#file888120line291 What's the motivation behind dropping the Aurora Updater here? Bill Farner wrote: I think the magic user value abuses a field that serves a different purpose. IMHO an API consumer should be able to programmatically determine that the scheduler independently performed an action without resorting to string matching on the magic value of a field. In this particular case, i don't think the user field adds signal to what is already present in the state enum value. Maxim Khutornenko wrote: I still think having a special Aurora Updater user clearly visible in the UI makes it way easier to grasp the event origin rather than trying to decipher a particular state meaning. Users may not and should not be familar with the update state diagram in order to understand the cause of the transition. This is especially true in a multi-actor environment where state transitions may come from users (pause/resume/abort), external monitoring service (pause) or the updater itself (pulse expired). I still think having a special Aurora Updater user clearly visible in the UI I completely agree, but the UI should make that decision, not the scheduler. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75726 --- On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 9, 2015, 5:50 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 291 https://reviews.apache.org/r/31814/diff/2/?file=888120#file888120line291 What's the motivation behind dropping the Aurora Updater here? I think the magic user value abuses a field that serves a different purpose. IMHO an API consumer should be able to programmatically determine that the scheduler independently performed an action without resorting to string matching on the magic value of a field. In this particular case, i don't think the user field adds signal to what is already present in the state enum value. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75726 --- On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31869: Catch only known Exception types in the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 9, 2015, 8:39 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31814: Include messages with internal job updater state transitions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 10, 2015, 12:35 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- Rebase. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 5d3b16a6e4f1d2af78ce70f85f6f0cc34829145f src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 0fe153c75455fb9a42361f48ffc7c384b831bb0f src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 10, 2015, 1:10 a.m., Aurora ReviewBot wrote: Master (e55113d) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.cli.plugins . SUCCESS src.test.python.apache.aurora.client.cli.quota . SUCCESS src.test.python.apache.aurora.client.cli.sla . SUCCESS src.test.python.apache.aurora.client.cli.supdate . SUCCESS src.test.python.apache.aurora.client.cli.task . SUCCESS src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.cli.version . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS src.test.python.apache.aurora.client.factory . SUCCESS src.test.python.apache.aurora.client.hooks.hooked_api . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS src.test.python.apache.aurora.common.test_aurora_job_key . SUCCESS src.test.python.apache.aurora.common.test_cluster . SUCCESS src.test.python.apache.aurora.common.test_cluster_option . SUCCESS src.test.python.apache.aurora.common.test_clusters . SUCCESS src.test.python.apache.aurora.common.test_http_signaler . SUCCESS src.test.python.apache.aurora.common.test_pex_version . SUCCESS src.test.python.apache.aurora.common.test_shellify . SUCCESS src.test.python.apache.aurora.common.test_transport . SUCCESS src.test.python.apache.aurora.config.test_base . SUCCESS src.test.python.apache.aurora.config.test_constraint_parsing . SUCCESS src.test.python.apache.aurora.config.test_loader . SUCCESS src.test.python.apache.aurora.config.test_thrift . SUCCESS src.test.python.apache.aurora.executor.common.path_detector . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . FAILURE src.test.python.apache.thermos.cli.commands.commands . SUCCESS src.test.python.apache.thermos.cli.common . SUCCESS src.test.python.apache.thermos.cli.main . SUCCESS src.test.python.apache.thermos.common.test_pathspec . SUCCESS src.test.python.apache.thermos.core.test_runner_integration . SUCCESS src.test.python.apache.thermos.monitoring.test_disk . SUCCESS FAILURE [31m FAILURE[0m I will refresh this build result if you post a review containing @ReviewBot retry Known flaky test: https://issues.apache.org/jira/browse/AURORA-1054 - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75832 --- On March 10, 2015, 12:35 a.m., Bill Farner wrote
Re: Review Request 31814: Include messages with internal job updater state transitions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75831 --- @ReviewBot retry - Bill Farner On March 10, 2015, 12:35 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 10, 2015, 12:35 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 5d3b16a6e4f1d2af78ce70f85f6f0cc34829145f src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 0fe153c75455fb9a42361f48ffc7c384b831bb0f src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 10, 2015, 12:54 a.m., Aurora ReviewBot wrote: Master (e55113d) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.cli.version . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS src.test.python.apache.aurora.client.factory . SUCCESS src.test.python.apache.aurora.client.hooks.hooked_api . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS src.test.python.apache.aurora.common.test_aurora_job_key . SUCCESS src.test.python.apache.aurora.common.test_cluster . SUCCESS src.test.python.apache.aurora.common.test_cluster_option . SUCCESS src.test.python.apache.aurora.common.test_clusters . SUCCESS src.test.python.apache.aurora.common.test_http_signaler . SUCCESS src.test.python.apache.aurora.common.test_pex_version . SUCCESS src.test.python.apache.aurora.common.test_shellify . SUCCESS src.test.python.apache.aurora.common.test_transport . SUCCESS src.test.python.apache.aurora.config.test_base . SUCCESS src.test.python.apache.aurora.config.test_constraint_parsing . SUCCESS src.test.python.apache.aurora.config.test_loader . SUCCESS src.test.python.apache.aurora.config.test_thrift . SUCCESS src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_detector . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . FAILURE src.test.python.apache.aurora.executor.common.path_detector . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS src.test.python.apache.thermos.cli.commands.commands . SUCCESS src.test.python.apache.thermos.cli.common . SUCCESS src.test.python.apache.thermos.cli.main . SUCCESS src.test.python.apache.thermos.common.test_pathspec . SUCCESS src.test.python.apache.thermos.core.test_runner_integration . SUCCESS src.test.python.apache.thermos.monitoring.test_disk . SUCCESS FAILURE [31m FAILURE[0m I will refresh this build result if you post a review containing @ReviewBot retry Created https://issues.apache.org/jira/browse/AURORA-1182 to track flaky test. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75829 --- On March 10, 2015, 12:35 a.m., Bill Farner wrote
Re: Review Request 31814: Include messages with internal job updater state transitions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75836 --- @ReviewBot retry - Bill Farner On March 10, 2015, 12:35 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 10, 2015, 12:35 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 5d3b16a6e4f1d2af78ce70f85f6f0cc34829145f src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 0fe153c75455fb9a42361f48ffc7c384b831bb0f src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31869: Catch only known Exception types in the client.
On March 9, 2015, 11:39 p.m., Kevin Sweeney wrote: src/test/python/apache/aurora/client/cli/test_create.py, line 277 https://reviews.apache.org/r/31869/diff/2/?file=889611#file889611line277 You don't actually need to use the pytest API for this - since we're on python 2.7 and you have already subclassed `TestCase` you can use [self.assertRaises](https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertRaises). I have no skin in this game, but i prefer to conform and minimize the ways of doing things. In this case, `pytest.raises` wins by the numbers: ``` $ grep -RI pytest.raises src/test/python/ | wc -l 69 $ grep -RI assertRaises src/test/python/ | wc -l 6 ``` On March 9, 2015, 11:39 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/base.py, lines 220-221 https://reviews.apache.org/r/31869/diff/2/?file=889607#file889607line220 To be clear my suggestion was to remove the constructor and leave the docstring. Overriding a constructor without changing its behavior just adds noise and is inconsistent with the convention used through most of the codebase. Aha, i didn't realize that `message` is a first-class property on `BaseException`. Done. On March 9, 2015, 11:39 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/client/base.py, line 217 https://reviews.apache.org/r/31869/diff/2/?file=889607#file889607line217 As this is only raised from a single place prefer declaring it there and matching the convention in [the style guide](https://github.com/twitter/commons/blob/master/src/python/twitter/common/styleguide.md#best-practices) (number 6). I don't have a strong opinion on the merits of this practice, but it is a convention across our code whose change I think is orthogonal to the purpose of this diff. Ok, i'll invert this so that `AuroraClientAPI.Error` is caught in `CommandLine`. As we discussed offline, this is a lesser abstraction violation. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/#review75812 --- On March 9, 2015, 8:39 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 9, 2015, 8:39 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
On March 7, 2015, 5:30 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, line 326 https://reviews.apache.org/r/31821/diff/1/?file=888148#file888148line326 Not yours, but it seems odd that we would call a function and internally suppress its behavior based on configuration. Seems like the caller should avoid the call if the behavior is not desired. If you agree, please TODO. Maxim Khutornenko wrote: Not sure I share your concern. This is a predicate intended to answer a simple question, which it does quite well. I doubt the alternative would gain us anything here. Perhaps i highlighted a line that created confusion. I'm suggesting that this predicate does not belong in this class, as it is used to determine whether to execute the 'body' of the preemptor. I suggest this predicate live in the caller. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/#review75637 --- On March 9, 2015, 11:34 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- (Updated March 9, 2015, 11:34 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 80c2023f46b63753dcec6a555dba626720a1925a src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImplTest.java 7207867813b0d096772dbc7f92fc1c76937e9831 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictimTest.java b0380b3fabb45be8ace55cfcf38ce15ef8040188 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 31828: Upgrade gradle and a few plugins.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31828/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Gradle 2.3: http://gradle.org/docs/2.3/release-notes Checkstyle: http://checkstyle.sourceforge.net/releasenotes.html Can't tell what, if anything, changed in the versions plugin: https://github.com/ben-manes/gradle-versions-plugin/releases Diffs - build.gradle b723118e84038a237b368ef4e3fe9549cd4b2854 buildSrc/gradle.properties 73341fa90f20d73edf34fc3c945b686dd14bf941 gradle/wrapper/gradle-wrapper.jar 2322723c7ed5f591adfa4c87b6c51932f591249a gradle/wrapper/gradle-wrapper.properties 531b41ced8b99d418636a18f4f97297722944ea3 Diff: https://reviews.apache.org/r/31828/diff/ Testing --- Thanks, Bill Farner
Review Request 31779: Change remaining update-related RPCs to use JobUpdateKey.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31779/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1093 https://issues.apache.org/jira/browse/AURORA-1093 Repository: aurora Description --- This was overlooked in AURORA-1093. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 1cd21e598db0c0d51cfed293e5e0fc51d84e0bb0 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 8ec5f9a3810b4deae981988487c6a46a20db72a2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 5989a62f1651aede6e2372ad3f519a9a947470de src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/python/apache/aurora/client/api/__init__.py c07122744e89fe61dbe4bea0c14400425983b2ef src/main/python/apache/aurora/client/cli/update.py a4890057b7d258926bc3dfb84fd1248e68051f31 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 824740856236976984d2114ec6a6aea989a87d1e src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 459f745cec1f85ece41376cade39c09254b50013 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/python/apache/aurora/client/api/test_api.py 0d552e8b9f9be300fc28a3f52aabe19e5a51b252 src/test/python/apache/aurora/client/cli/test_create.py a65aab71ee8ce19dc2c05ea230258084d6f55727 src/test/python/apache/aurora/client/cli/test_restart.py b596babc3c6877f68094943126b8cd49be3fc635 src/test/python/apache/aurora/client/cli/test_supdate.py 93a5532dc6f7aee2c40bc86385a630b9a1b6f528 src/test/python/apache/aurora/client/cli/util.py 6d3cc51f50b417405549c254531c854565a54949 Diff: https://reviews.apache.org/r/31779/diff/ Testing --- Normal test suite + end-to-end tests. Thanks, Bill Farner
Re: Review Request 31570: Suppressing duplicate update instance events.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31570/#review75224 --- Ship it! Ship It! - Bill Farner On Feb. 28, 2015, 1:37 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31570/ --- (Updated Feb. 28, 2015, 1:37 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1097 https://issues.apache.org/jira/browse/AURORA-1097 Repository: aurora Description --- All instance update actions are supposed to be unique. Suppressing duplicats due to pause/resume cycle. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa Diff: https://reviews.apache.org/r/31570/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
