date:20140917


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25760/#review53786
---

Ship it!


LGTM mod Maxim's comments.

- Bill Farner


On Sept. 18, 2014, 12:20 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25760/
> ---
> 
> (Updated Sept. 18, 2014, 12:20 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-722
> https://issues.apache.org/jira/browse/AURORA-722
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> * Deflate snapshots using stream API
> * Make LogManager non-final
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> da4f0e5520f85782bbc246752cde29bd279be466 
>   src/main/java/org/apache/aurora/scheduler/storage/log/Entries.java 
> 1eca768a7daf6defd4bb35a5c46e9c0eab370d92 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
> 0b59043cf5f01c99d09168d7669f51b686d2e930 
>   src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
> 7aaab1e7debbfed65ac7c15154ec73fc9f2114af 
> 
> Diff: https://reviews.apache.org/r/25760/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> The StreamManager @Timed annotations don't work yet since guice isn't used to 
> instantiate it. Ideally we'd use AssistedInject here, but that's a slightly 
> larger change.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 25760: Performance improvements and instrumentation for snapshot


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25760/#review53778
---

Ship it!



src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java


Static import?



src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java


Technically, the outBytes may already be disposed at this point. Move 
return inside of outer try()?



src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java


Line break after '=' instead?



src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java


Now you have to add jsmith to reviewers :)


- Maxim Khutornenko


On Sept. 18, 2014, 12:20 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25760/
> ---
> 
> (Updated Sept. 18, 2014, 12:20 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-722
> https://issues.apache.org/jira/browse/AURORA-722
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> * Deflate snapshots using stream API
> * Make LogManager non-final
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> da4f0e5520f85782bbc246752cde29bd279be466 
>   src/main/java/org/apache/aurora/scheduler/storage/log/Entries.java 
> 1eca768a7daf6defd4bb35a5c46e9c0eab370d92 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
> 0b59043cf5f01c99d09168d7669f51b686d2e930 
>   src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
> 7aaab1e7debbfed65ac7c15154ec73fc9f2114af 
> 
> Diff: https://reviews.apache.org/r/25760/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> The StreamManager @Timed annotations don't work yet since guice isn't used to 
> instantiate it. Ideally we'd use AssistedInject here, but that's a slightly 
> larger change.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Review Request 25760: Performance improvements and instrumentation for snapshot

2014-09-17 Thread Kevin Sweeney


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25760/
---

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Bugs: AURORA-722
https://issues.apache.org/jira/browse/AURORA-722


Repository: aurora


Description
---

* Deflate snapshots using stream API
* Make LogManager non-final


Diffs
-

  src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
da4f0e5520f85782bbc246752cde29bd279be466 
  src/main/java/org/apache/aurora/scheduler/storage/log/Entries.java 
1eca768a7daf6defd4bb35a5c46e9c0eab370d92 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
0b59043cf5f01c99d09168d7669f51b686d2e930 
  src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
7aaab1e7debbfed65ac7c15154ec73fc9f2114af 

Diff: https://reviews.apache.org/r/25760/diff/


Testing
---

./gradlew -Pq build

The StreamManager @Timed annotations don't work yet since guice isn't used to 
instantiate it. Ideally we'd use AssistedInject here, but that's a slightly 
larger change.


Thanks,

Kevin Sweeney

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig



> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote:
> > +1 to the code change, UI stuff in particular looks good to me.
> > 
> > -1 to dropping instanceCount completely though. I thought we mentioned we 
> > wanted to capture and store all the original details the user sends? Even 
> > if this is purely for auditing and never used internally, I still think 
> > it's useful.
> 
> Bill Farner wrote:
> Should we change direction a bit and just store the original 
> JobUpdateRequest?
> 
> Maxim Khutornenko wrote:
> Storing instance count is completely redundant and prone to integrity 
> problems (i.e. need to make sure instance ranges match instance counts).
> 
> Maxim Khutornenko wrote:
> | Should we change direction a bit and just store the original 
> JobUpdateRequest
> Don't really see what it would buy us. All that data is already available 
> in the current schema.
> 
> David McLaughlin wrote:
> Instance count is not instanceRanges.length, instead it's an option the 
> user sent as part of the update - the value we were already storing. It's 
> redundant once the instance ranges have been calculated but it is used to 
> make that calculation the first time (in the absence of 
> updateOnlyTheseInstances). I think it's useful for auditing and figuring out 
> why the scheduler made the decision it did.

We already log all thrift requests. This should be enough to restore audit 
trail when needed:

Sep 17, 2014 3:46:07 PM 
org.apache.aurora.scheduler.thrift.aop.LoggingInterceptor invoke
INFO: startJobUpdate(JobUpdateRequest(jobKey:JobKey(role:bar_role, 
environment:devel, name:job_foo), 
taskConfig:TaskConfig(owner:Identity(role:bar_role, user:foo_user), 
environment:devel, jobName:job_foo, isService:false, numCpus:1.0, ramMb:8, 
diskMb:1024, priority:0, maxTaskFailures:1, production:true, 
constraints:[Constraint(name:host, constraint:)], requestedPorts:[], taskLinks:{}, 
contactEmail:test...@twitter.com, executorConfig:ExecutorConfig(name:aurora, 
data:data)), **instanceCount:6**, settings:JobUpdateSettings(updateGroupSize:0, 
maxPerInstanceFailures:0, maxFailedInstances:0, maxWaitToInstanceRunningMs:0, 
minWaitInInstanceRunningMs:0, rollbackOnFailure:false, 
updateOnlyTheseInstances:null)), foo_user)


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25750/#review53746
---


On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25750/
> ---
> 
> (Updated Sept. 17, 2014, 8:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-717
> https://issues.apache.org/jira/browse/AURORA-717
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Converted newTaskConfig into InstanceTaskConfig to allow multiple instance 
> ranges.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 5ac42b6860a1e99f27b6a4067d370f26943f9212 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> dfaadc8cf6bf1d929e4e5fec8347a804c6478122 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  04a9246467ce140300b3b543bdb98ad4fe8302ff 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d4b8141d9d483a21d18afd9c6fbb2cf639595101 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> 25382910704f86e6ca292c7f8eae5990663c4b46 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html 
> 613b5325a3ae53fa61e6bac58bcc6e76950f7031 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> a450a090f76fe565924e2f9c5340c10d1f6f05be 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig



> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote:
> > +1 to the code change, UI stuff in particular looks good to me.
> > 
> > -1 to dropping instanceCount completely though. I thought we mentioned we 
> > wanted to capture and store all the original details the user sends? Even 
> > if this is purely for auditing and never used internally, I still think 
> > it's useful.
> 
> Bill Farner wrote:
> Should we change direction a bit and just store the original 
> JobUpdateRequest?
> 
> Maxim Khutornenko wrote:
> Storing instance count is completely redundant and prone to integrity 
> problems (i.e. need to make sure instance ranges match instance counts).
> 
> Maxim Khutornenko wrote:
> | Should we change direction a bit and just store the original 
> JobUpdateRequest
> Don't really see what it would buy us. All that data is already available 
> in the current schema.

Instance count is not instanceRanges.length, instead it's an option the user 
sent as part of the update - the value we were already storing. It's redundant 
once the instance ranges have been calculated but it is used to make that 
calculation the first time (in the absence of updateOnlyTheseInstances). I 
think it's useful for auditing and figuring out why the scheduler made the 
decision it did.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25750/#review53746
---


On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25750/
> ---
> 
> (Updated Sept. 17, 2014, 8:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-717
> https://issues.apache.org/jira/browse/AURORA-717
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Converted newTaskConfig into InstanceTaskConfig to allow multiple instance 
> ranges.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 5ac42b6860a1e99f27b6a4067d370f26943f9212 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> dfaadc8cf6bf1d929e4e5fec8347a804c6478122 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  04a9246467ce140300b3b543bdb98ad4fe8302ff 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d4b8141d9d483a21d18afd9c6fbb2cf639595101 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> 25382910704f86e6ca292c7f8eae5990663c4b46 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html 
> 613b5325a3ae53fa61e6bac58bcc6e76950f7031 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> a450a090f76fe565924e2f9c5340c10d1f6f05be 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  7d4dd3720ed946c1dd10b9f3979ded796fb15d98 
>   src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 
> 1b8e5c2c9e21810589b6770129f742de4f1a67e2 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> f698b532a3827e59e654d6e07e20f5725aed6768 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> c5838761783d85a547688d4f708a75c1fd240201 
> 
> Diff: https://reviews.apache.org/r/25750/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 25753: Surface instance update status changes so they may be persisted.


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25753/#review53753
---

Ship it!


Other than the docs, lgtm.

- David McLaughlin


On Sept. 17, 2014, 8:55 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25753/
> ---
> 
> (Updated Sept. 17, 2014, 8:55 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Rework outputs from OneWayJobUpdater so JobUpdateControllerImpl has 
> visibility into internal state machine transitions, which is the last bit 
> necessary before we can start saving instance update events.
> 
> Also contains a small bit of cleanup:
> - removed TODOs that have been addressed
> - made Optional a member of Result, rather than having a 
> switch to map between them.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> dc11abda72855f925a93913f57c5d0e15bc2e0ff 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 5a9af39ca619b90dfbac16b8fed55a3f7127cce3 
>   src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> 453daeb0b1bb6c9d958e0265d0b7379e1b7a6558 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/test/java/org/apache/aurora/scheduler/updater/EnumsTest.java 
> defdf6ea056b6524a8fc25f76be37f9ed0fad3e8 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 6169471388eabc83245fbe3b8abb2c3a38eb00e1 
> 
> Diff: https://reviews.apache.org/r/25753/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig



> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote:
> > +1 to the code change, UI stuff in particular looks good to me.
> > 
> > -1 to dropping instanceCount completely though. I thought we mentioned we 
> > wanted to capture and store all the original details the user sends? Even 
> > if this is purely for auditing and never used internally, I still think 
> > it's useful.
> 
> Bill Farner wrote:
> Should we change direction a bit and just store the original 
> JobUpdateRequest?
> 
> Maxim Khutornenko wrote:
> Storing instance count is completely redundant and prone to integrity 
> problems (i.e. need to make sure instance ranges match instance counts).

| Should we change direction a bit and just store the original JobUpdateRequest
Don't really see what it would buy us. All that data is already available in 
the current schema.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25750/#review53746
---


On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25750/
> ---
> 
> (Updated Sept. 17, 2014, 8:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-717
> https://issues.apache.org/jira/browse/AURORA-717
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Converted newTaskConfig into InstanceTaskConfig to allow multiple instance 
> ranges.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 5ac42b6860a1e99f27b6a4067d370f26943f9212 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> dfaadc8cf6bf1d929e4e5fec8347a804c6478122 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  04a9246467ce140300b3b543bdb98ad4fe8302ff 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d4b8141d9d483a21d18afd9c6fbb2cf639595101 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> 25382910704f86e6ca292c7f8eae5990663c4b46 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html 
> 613b5325a3ae53fa61e6bac58bcc6e76950f7031 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> a450a090f76fe565924e2f9c5340c10d1f6f05be 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  7d4dd3720ed946c1dd10b9f3979ded796fb15d98 
>   src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 
> 1b8e5c2c9e21810589b6770129f742de4f1a67e2 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> f698b532a3827e59e654d6e07e20f5725aed6768 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> c5838761783d85a547688d4f708a75c1fd240201 
> 
> Diff: https://reviews.apache.org/r/25750/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig



> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote:
> > +1 to the code change, UI stuff in particular looks good to me.
> > 
> > -1 to dropping instanceCount completely though. I thought we mentioned we 
> > wanted to capture and store all the original details the user sends? Even 
> > if this is purely for auditing and never used internally, I still think 
> > it's useful.
> 
> Bill Farner wrote:
> Should we change direction a bit and just store the original 
> JobUpdateRequest?

Storing instance count is completely redundant and prone to integrity problems 
(i.e. need to make sure instance ranges match instance counts).


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25750/#review53746
---


On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25750/
> ---
> 
> (Updated Sept. 17, 2014, 8:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-717
> https://issues.apache.org/jira/browse/AURORA-717
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Converted newTaskConfig into InstanceTaskConfig to allow multiple instance 
> ranges.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 5ac42b6860a1e99f27b6a4067d370f26943f9212 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> dfaadc8cf6bf1d929e4e5fec8347a804c6478122 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  04a9246467ce140300b3b543bdb98ad4fe8302ff 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d4b8141d9d483a21d18afd9c6fbb2cf639595101 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> 25382910704f86e6ca292c7f8eae5990663c4b46 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html 
> 613b5325a3ae53fa61e6bac58bcc6e76950f7031 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> a450a090f76fe565924e2f9c5340c10d1f6f05be 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  7d4dd3720ed946c1dd10b9f3979ded796fb15d98 
>   src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 
> 1b8e5c2c9e21810589b6770129f742de4f1a67e2 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> f698b532a3827e59e654d6e07e20f5725aed6768 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> c5838761783d85a547688d4f708a75c1fd240201 
> 
> Diff: https://reviews.apache.org/r/25750/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig



> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote:
> > +1 to the code change, UI stuff in particular looks good to me.
> > 
> > -1 to dropping instanceCount completely though. I thought we mentioned we 
> > wanted to capture and store all the original details the user sends? Even 
> > if this is purely for auditing and never used internally, I still think 
> > it's useful.

Should we change direction a bit and just store the original JobUpdateRequest?


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25750/#review53746
---


On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25750/
> ---
> 
> (Updated Sept. 17, 2014, 8:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-717
> https://issues.apache.org/jira/browse/AURORA-717
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Converted newTaskConfig into InstanceTaskConfig to allow multiple instance 
> ranges.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 5ac42b6860a1e99f27b6a4067d370f26943f9212 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> dfaadc8cf6bf1d929e4e5fec8347a804c6478122 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  04a9246467ce140300b3b543bdb98ad4fe8302ff 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d4b8141d9d483a21d18afd9c6fbb2cf639595101 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> 25382910704f86e6ca292c7f8eae5990663c4b46 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html 
> 613b5325a3ae53fa61e6bac58bcc6e76950f7031 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> a450a090f76fe565924e2f9c5340c10d1f6f05be 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  7d4dd3720ed946c1dd10b9f3979ded796fb15d98 
>   src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 
> 1b8e5c2c9e21810589b6770129f742de4f1a67e2 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> f698b532a3827e59e654d6e07e20f5725aed6768 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> c5838761783d85a547688d4f708a75c1fd240201 
> 
> Diff: https://reviews.apache.org/r/25750/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25750/#review53746
---


+1 to the code change, UI stuff in particular looks good to me.

-1 to dropping instanceCount completely though. I thought we mentioned we 
wanted to capture and store all the original details the user sends? Even if 
this is purely for auditing and never used internally, I still think it's 
useful.

- David McLaughlin


On Sept. 17, 2014, 8:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25750/
> ---
> 
> (Updated Sept. 17, 2014, 8:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-717
> https://issues.apache.org/jira/browse/AURORA-717
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Converted newTaskConfig into InstanceTaskConfig to allow multiple instance 
> ranges.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 5ac42b6860a1e99f27b6a4067d370f26943f9212 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> dfaadc8cf6bf1d929e4e5fec8347a804c6478122 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 0737f92bc3ea971c64ca6d84b02da7e2b5b934bf 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  04a9246467ce140300b3b543bdb98ad4fe8302ff 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  d4b8141d9d483a21d18afd9c6fbb2cf639595101 
>   
> src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
> 25382910704f86e6ca292c7f8eae5990663c4b46 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html 
> 613b5325a3ae53fa61e6bac58bcc6e76950f7031 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> a450a090f76fe565924e2f9c5340c10d1f6f05be 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  7d4dd3720ed946c1dd10b9f3979ded796fb15d98 
>   src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 
> 1b8e5c2c9e21810589b6770129f742de4f1a67e2 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> f698b532a3827e59e654d6e07e20f5725aed6768 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> c5838761783d85a547688d4f708a75c1fd240201 
> 
> Diff: https://reviews.apache.org/r/25750/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 25753: Surface instance update status changes so they may be persisted.


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25753/#review53743
---

Ship it!



src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java


Static import for SideEffect.InstanceUpdateStatus.WORKING?



src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java


Missing xml docs for public methods.



src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java


Document?


- Maxim Khutornenko


On Sept. 17, 2014, 8:55 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25753/
> ---
> 
> (Updated Sept. 17, 2014, 8:55 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Rework outputs from OneWayJobUpdater so JobUpdateControllerImpl has 
> visibility into internal state machine transitions, which is the last bit 
> necessary before we can start saving instance update events.
> 
> Also contains a small bit of cleanup:
> - removed TODOs that have been addressed
> - made Optional a member of Result, rather than having a 
> switch to map between them.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> dc11abda72855f925a93913f57c5d0e15bc2e0ff 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  159b09e7c00175bf3aea893d48cb3953186bd6cb 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 5a9af39ca619b90dfbac16b8fed55a3f7127cce3 
>   src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
> 453daeb0b1bb6c9d958e0265d0b7379e1b7a6558 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
>   src/test/java/org/apache/aurora/scheduler/updater/EnumsTest.java 
> defdf6ea056b6524a8fc25f76be37f9ed0fad3e8 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> 6169471388eabc83245fbe3b8abb2c3a38eb00e1 
> 
> Diff: https://reviews.apache.org/r/25753/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25753: Surface instance update status changes so they may be persisted.


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25753/
---

(Updated Sept. 17, 2014, 8:55 p.m.)


Review request for Aurora, David McLaughlin and Maxim Khutornenko.


Repository: aurora


Description
---

Rework outputs from OneWayJobUpdater so JobUpdateControllerImpl has visibility 
into internal state machine transitions, which is the last bit necessary before 
we can start saving instance update events.

Also contains a small bit of cleanup:
- removed TODOs that have been addressed
- made Optional a member of Result, rather than having a switch 
to map between them.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
dc11abda72855f925a93913f57c5d0e15bc2e0ff 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
159b09e7c00175bf3aea893d48cb3953186bd6cb 
  src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
5a9af39ca619b90dfbac16b8fed55a3f7127cce3 
  src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
453daeb0b1bb6c9d958e0265d0b7379e1b7a6558 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
  src/test/java/org/apache/aurora/scheduler/updater/EnumsTest.java 
defdf6ea056b6524a8fc25f76be37f9ed0fad3e8 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
6169471388eabc83245fbe3b8abb2c3a38eb00e1 

Diff: https://reviews.apache.org/r/25753/diff/


Testing
---

./gradlew build -Pq


Thanks,

Bill Farner

Review Request 25753: Surface instance update status changes so they may be persisted.


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25753/
---

Review request for Aurora, David McLaughlin and Maxim Khutornenko.


Repository: aurora


Description
---

Rework outputs from OneWayJobUpdater so JobUpdateControllerImpl has visibility 
into internal state machine transitions, which is the last bit necessary before 
we can start saving instance update events.

Also contains a small bit of cleanup:
- removed TODOs that have been addressed
- made Optional a member of Result, rather than having a switch 
to map between them.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
dc11abda72855f925a93913f57c5d0e15bc2e0ff 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
159b09e7c00175bf3aea893d48cb3953186bd6cb 
  src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
5a9af39ca619b90dfbac16b8fed55a3f7127cce3 
  src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 
453daeb0b1bb6c9d958e0265d0b7379e1b7a6558 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
  src/test/java/org/apache/aurora/scheduler/updater/EnumsTest.java 
defdf6ea056b6524a8fc25f76be37f9ed0fad3e8 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
6169471388eabc83245fbe3b8abb2c3a38eb00e1 

Diff: https://reviews.apache.org/r/25753/diff/


Testing
---

./gradlew build -Pq


Thanks,

Bill Farner

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25750/
---

(Updated Sept. 17, 2014, 8:40 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
---

CR comments.


Bugs: AURORA-717
https://issues.apache.org/jira/browse/AURORA-717


Repository: aurora


Description
---

Converted newTaskConfig into InstanceTaskConfig to allow multiple instance 
ranges.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
5ac42b6860a1e99f27b6a4067d370f26943f9212 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
dfaadc8cf6bf1d929e4e5fec8347a804c6478122 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
0737f92bc3ea971c64ca6d84b02da7e2b5b934bf 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 04a9246467ce140300b3b543bdb98ad4fe8302ff 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
d4b8141d9d483a21d18afd9c6fbb2cf639595101 
  src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java 
25382910704f86e6ca292c7f8eae5990663c4b46 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
159b09e7c00175bf3aea893d48cb3953186bd6cb 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
82f2b6d98e8270efb9b6517b1f9782a8c5a9aa39 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
5d6299f9de6eccd0f1332e11d57dfb910d956011 
  src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html 
613b5325a3ae53fa61e6bac58bcc6e76950f7031 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 0e7f6ebd35a6dee28a16e28fa7b10b10d20c70c5 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
a450a090f76fe565924e2f9c5340c10d1f6f05be 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
e09caa63bc0150d7109cb237e80b9efee441dded 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 7d4dd3720ed946c1dd10b9f3979ded796fb15d98 
  src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 
1b8e5c2c9e21810589b6770129f742de4f1a67e2 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
0e15a79ae6dc877dfa6ba492efafdf1f89f7d40f 
  src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
f698b532a3827e59e654d6e07e20f5725aed6768 
  src/test/resources/org/apache/aurora/gen/api.thrift.md5 
c5838761783d85a547688d4f708a75c1fd240201 

Diff: https://reviews.apache.org/r/25750/diff/


Testing
---

gradle -Pq build
./pants src/test/python:all


Thanks,

Maxim Khutornenko

Re: Review Request 25721: Asynchronous JS for Scheduler UI

2014-09-17 Thread Joshua Cohen


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53733
---

Ship it!


Ship It!

- Joshua Cohen


On Sept. 17, 2014, 5:49 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25721/
> ---
> 
> (Updated Sept. 17, 2014, 5:49 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-700
> https://issues.apache.org/jira/browse/AURORA-700
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Asynchronous JS for Scheduler UI.
> 
> I have tried to change the minimum amount of JavaScript to keep this review 
> small, even though doing this made me really want to tear everything up and 
> start again :-)
> 
> I attached two screenshots to show the sync vs async behaviour in the browser 
> - note that the async version is 2x the latency of the first. This is because 
> the getJobSummary requests is 10KB compared to <1KB in the sync version. The 
> point is how work is done in parallel. 
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/job.html 
> 14dce65158eab83906c68f9afabf49e39283287d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> c80146aa3829e3c3102645a1864dbeaf5e2e56bc 
> 
> Diff: https://reviews.apache.org/r/25721/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> I did manual testing to verify I didn't accidentally introduce any 
> regressions. I have around 80% confidence there are no regressions here, 
> mainly because I wasted an hour on totally unintuitive behaviour from 
> SmartTable. So I'm going to do a bunch more testing, which will involve mocks 
> for updates and crons.
> 
> 
> File Attachments
> 
> 
> Before: Synchronous, serial evaluation of network requests.
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/17/ce60917a-5c25-4600-8c1f-cc816aa96a5e__before-sync.png
> AFTER: Asynchronous, parallel network requests.
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/17/dc703579-7072-4aa0-b51a-6df99521fcd5__after-async.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

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

2014-09-17 Thread Mark Chu-Carroll


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25255/
---

(Updated Sept. 17, 2014, 2:44 p.m.)


Review request for Aurora.


Changes
---

rebase to master.


Repository: aurora


Description
---

This change contains the basic commands needed to work with the
scheduler-driven updater. (It does not yet cover querying for the status
of the update; that will come in a subsequent change.)


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/BUILD 
4ed8f8a3e29010282c5cb0cc461980bcfc6ae478 
  src/main/python/apache/aurora/client/cli/context.py 
51c7d24dca664e476e62f1864d095416dfab70e4 
  src/main/python/apache/aurora/client/cli/jobs.py 
aeb78303fc2071ccad848b1de4512e8ca585bf06 
  src/main/python/apache/aurora/client/cli/standalone_client.py 
b7c8de66d6e4664b536911f826e36a984e8d0fef 
  src/main/python/apache/aurora/client/cli/update.py PRE-CREATION 
  src/test/python/apache/aurora/client/cli/BUILD 
70e704d2a3c334eaa8ba905eb3c75c2e9ee152bc 
  src/test/python/apache/aurora/client/cli/test_restart.py 
377ecfe6180785e57a389f6e1bc8b184bca0dd6c 
  src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION 
  src/test/python/apache/aurora/client/cli/util.py 
ebabe529966cea503f11404f37961c5d577f00b7 

Diff: https://reviews.apache.org/r/25255/diff/


Testing
---

New suite of tests for the new command; all unit tests pass.


Thanks,

Mark Chu-Carroll

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25741/#review53708
---

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25741/
> ---
> 
> (Updated Sept. 17, 2014, 5:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
> `JobUpdateInstructions`) to represent the delta incurred by a job update 
> (AURORA-718), i believe we can remove these two states, as addition/removal 
> can be inferred from the delta.
> 
> Furthermore, this allows the job updater's internal state machine to 
> translate directly into the remaining `JobUpdateActions`, and hides details 
> of the non-atomic actions necessary to move an instance from state A to state 
> B.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
> 
> Diff: https://reviews.apache.org/r/25741/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25741/#review53706
---

Ship it!


Ship It!

- David McLaughlin


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25741/
> ---
> 
> (Updated Sept. 17, 2014, 5:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
> `JobUpdateInstructions`) to represent the delta incurred by a job update 
> (AURORA-718), i believe we can remove these two states, as addition/removal 
> can be inferred from the delta.
> 
> Furthermore, this allows the job updater's internal state machine to 
> translate directly into the remaining `JobUpdateActions`, and hides details 
> of the non-atomic actions necessary to move an instance from state A to state 
> B.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
> 
> Diff: https://reviews.apache.org/r/25741/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.



> On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote:
> > Does removing an instance take any significant amount of time? Can it fail? 
> > The concern is we'll be showing 100% progress but the update is still 
> > running when reducing the instance count.
> 
> Bill Farner wrote:
> Depends on your definition of significant.  It does take a 
> non-deterministic amount of time, since we transition to KILLING and await to 
> see that the task is gone.  It can _sort of_ fail, but not in a way that we 
> will record an instance failure AFAICT.  However, i think the behavior of 
> ACTING -> FINISHED even in removal is nice as it matches two-step nature of 
> the operation.
> 
> David McLaughlin wrote:
> I see, so the new behaviour is that in both scenarios the instance would 
> end up in INSTANCE_UPDATED state? And then the client infers whether it was 
> added or removed based on the instance count delta?

Effectively, yes.  The states communicate "acting" and "finished" along with 
direction (updating or rollback), and the caller has the information to 
determine what the outcome looks like (config change, added, removed).


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25741/#review53694
---


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25741/
> ---
> 
> (Updated Sept. 17, 2014, 5:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
> `JobUpdateInstructions`) to represent the delta incurred by a job update 
> (AURORA-718), i believe we can remove these two states, as addition/removal 
> can be inferred from the delta.
> 
> Furthermore, this allows the job updater's internal state machine to 
> translate directly into the remaining `JobUpdateActions`, and hides details 
> of the non-atomic actions necessary to move an instance from state A to state 
> B.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
> 
> Diff: https://reviews.apache.org/r/25741/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.



> On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote:
> > Does removing an instance take any significant amount of time? Can it fail? 
> > The concern is we'll be showing 100% progress but the update is still 
> > running when reducing the instance count.
> 
> Bill Farner wrote:
> Depends on your definition of significant.  It does take a 
> non-deterministic amount of time, since we transition to KILLING and await to 
> see that the task is gone.  It can _sort of_ fail, but not in a way that we 
> will record an instance failure AFAICT.  However, i think the behavior of 
> ACTING -> FINISHED even in removal is nice as it matches two-step nature of 
> the operation.

I see, so the new behaviour is that in both scenarios the instance would end up 
in INSTANCE_UPDATED state? And then the client infers whether it was added or 
removed based on the instance count delta?


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25741/#review53694
---


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25741/
> ---
> 
> (Updated Sept. 17, 2014, 5:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
> `JobUpdateInstructions`) to represent the delta incurred by a job update 
> (AURORA-718), i believe we can remove these two states, as addition/removal 
> can be inferred from the delta.
> 
> Furthermore, this allows the job updater's internal state machine to 
> translate directly into the remaining `JobUpdateActions`, and hides details 
> of the non-atomic actions necessary to move an instance from state A to state 
> B.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
> 
> Diff: https://reviews.apache.org/r/25741/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25721: Asynchronous JS for Scheduler UI



> On Sept. 17, 2014, 4:34 a.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, 
> > line 368
> > 
> >
> > More objective?  I don't want to kill the humor, but an objective 
> > statement will be more useful to the next developer.
> > 
> > Also, would it make sense to drop ao TODO to add a spinner or something 
> > rather than empty data?  This may be obliterated by an overhaul, but making 
> > note that this is suboptimal user experience would be a plus.

I added a little "loading job information" div while the tasks query executes, 
because that's the main thing that might confuse users. Filed AURORA-721 for 
solving this for all network requests.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/#review53651
---


On Sept. 17, 2014, 5:49 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25721/
> ---
> 
> (Updated Sept. 17, 2014, 5:49 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-700
> https://issues.apache.org/jira/browse/AURORA-700
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Asynchronous JS for Scheduler UI.
> 
> I have tried to change the minimum amount of JavaScript to keep this review 
> small, even though doing this made me really want to tear everything up and 
> start again :-)
> 
> I attached two screenshots to show the sync vs async behaviour in the browser 
> - note that the async version is 2x the latency of the first. This is because 
> the getJobSummary requests is 10KB compared to <1KB in the sync version. The 
> point is how work is done in parallel. 
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/job.html 
> 14dce65158eab83906c68f9afabf49e39283287d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> c80146aa3829e3c3102645a1864dbeaf5e2e56bc 
> 
> Diff: https://reviews.apache.org/r/25721/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> I did manual testing to verify I didn't accidentally introduce any 
> regressions. I have around 80% confidence there are no regressions here, 
> mainly because I wasted an hour on totally unintuitive behaviour from 
> SmartTable. So I'm going to do a bunch more testing, which will involve mocks 
> for updates and crons.
> 
> 
> File Attachments
> 
> 
> Before: Synchronous, serial evaluation of network requests.
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/17/ce60917a-5c25-4600-8c1f-cc816aa96a5e__before-sync.png
> AFTER: Asynchronous, parallel network requests.
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/17/dc703579-7072-4aa0-b51a-6df99521fcd5__after-async.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.



> On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote:
> > Does removing an instance take any significant amount of time? Can it fail? 
> > The concern is we'll be showing 100% progress but the update is still 
> > running when reducing the instance count.

Depends on your definition of significant.  It does take a non-deterministic 
amount of time, since we transition to KILLING and await to see that the task 
is gone.  It can _sort of_ fail, but not in a way that we will record an 
instance failure AFAICT.  However, i think the behavior of ACTING -> FINISHED 
even in removal is nice as it matches two-step nature of the operation.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25741/#review53694
---


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25741/
> ---
> 
> (Updated Sept. 17, 2014, 5:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
> `JobUpdateInstructions`) to represent the delta incurred by a job update 
> (AURORA-718), i believe we can remove these two states, as addition/removal 
> can be inferred from the delta.
> 
> Furthermore, this allows the job updater's internal state machine to 
> translate directly into the remaining `JobUpdateActions`, and hides details 
> of the non-atomic actions necessary to move an instance from state A to state 
> B.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
> 
> Diff: https://reviews.apache.org/r/25741/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 25721: Asynchronous JS for Scheduler UI

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25721/
---

(Updated Sept. 17, 2014, 5:49 p.m.)

Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner.

Changes
---

RB feedback.

Bugs: AURORA-700
https://issues.apache.org/jira/browse/AURORA-700

Repository: aurora

Description
---

Asynchronous JS for Scheduler UI.

I have tried to change the minimum amount of JavaScript to keep this review
small, even though doing this made me really want to tear everything up and
start again :-)

I attached two screenshots to show the sync vs async behaviour in the browser -
note that the async version is 2x the latency of the first. This is because the
getJobSummary requests is 10KB compared to <1KB in the sync version. The point
is how work is done in parallel.

Diffs (updated)
-

src/main/resources/org/apache/aurora/scheduler/http/ui/job.html
14dce65158eab83906c68f9afabf49e39283287d
src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js
0884cc8f0504a953ef694dae0e6b05ba6e2bff61
src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js
c80146aa3829e3c3102645a1864dbeaf5e2e56bc

Diff: https://reviews.apache.org/r/25721/diff/

Testing
---

./gradlew jsHint

I did manual testing to verify I didn't accidentally introduce any regressions.
I have around 80% confidence there are no regressions here, mainly because I
wasted an hour on totally unintuitive behaviour from SmartTable. So I'm going
to do a bunch more testing, which will involve mocks for updates and crons.

File Attachments

Before: Synchronous, serial evaluation of network requests.

https://reviews.apache.org/media/uploaded/files/2014/09/17/ce60917a-5c25-4600-8c1f-cc816aa96a5e__before-sync.png
AFTER: Asynchronous, parallel network requests.

https://reviews.apache.org/media/uploaded/files/2014/09/17/dc703579-7072-4aa0-b51a-6df99521fcd5__after-async.png

Thanks,

David McLaughlin

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25741/#review53694
---


Does removing an instance take any significant amount of time? Can it fail? The 
concern is we'll be showing 100% progress but the update is still running when 
reducing the instance count.

- David McLaughlin


On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25741/
> ---
> 
> (Updated Sept. 17, 2014, 5:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Since we're changing fields in `JobUpdateConfiguration` (being renamed to 
> `JobUpdateInstructions`) to represent the delta incurred by a job update 
> (AURORA-718), i believe we can remove these two states, as addition/removal 
> can be inferred from the delta.
> 
> Furthermore, this allows the job updater's internal state machine to 
> translate directly into the remaining `JobUpdateActions`, and hides details 
> of the non-atomic actions necessary to move an instance from state A to state 
> B.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 5d6299f9de6eccd0f1332e11d57dfb910d956011 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  e09caa63bc0150d7109cb237e80b9efee441dded 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> ca990e73d80e8456e71a97f0bdd0b6f4530d0135 
> 
> Diff: https://reviews.apache.org/r/25741/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

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


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25255/#review53693
---

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 17, 2014, 5:06 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25255/
> ---
> 
> (Updated Sept. 17, 2014, 5:06 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This change contains the basic commands needed to work with the
> scheduler-driven updater. (It does not yet cover querying for the status
> of the update; that will come in a subsequent change.)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/BUILD 
> ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 
>   src/main/python/apache/aurora/client/cli/context.py 
> 51c7d24dca664e476e62f1864d095416dfab70e4 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> ebc22aaa5a8aed311897b3ce9632b6f7175b6080 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> b7c8de66d6e4664b536911f826e36a984e8d0fef 
>   src/main/python/apache/aurora/client/cli/update.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/BUILD 
> e1f9ebf96774b8f5c75de8570c6ba87d953ab649 
>   src/test/python/apache/aurora/client/cli/test_restart.py 
> a1e7a5a94a2d336239df98e2600658b97c546901 
>   src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION 
>   src/test/python/apache/aurora/client/cli/util.py 
> 95a2123e127c9811fd2305e71cfc5c7c4376f904 
> 
> Diff: https://reviews.apache.org/r/25255/diff/
> 
> 
> Testing
> ---
> 
> New suite of tests for the new command; all unit tests pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.