> 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
> 
>

Reply via email to