Re: Review Request 26762: Preparing for Identity struct deprecation (scheduler).

2014-10-23 Thread Maxim Khutornenko

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

(Updated Oct. 23, 2014, 10:06 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Minor comment changes.


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


Repository: aurora


Description
---

This patch replaces the Identity struct use within the scheduler and makes it 
ready for removal (i.e. when the JobStore is ready). 

Sending it as a separate CR for easier reviewing. 

Will have to be committed along with python changes 
(https://reviews.apache.org/r/26954/) to avoid breaking client diff 
functionality.

Summary of the changes:
* TaskConfig - dual write in StorageBackfill to populate new _key_ field. 
Incoming thrift objects are populated in ConfigurationManager during sanitizing.
* TaskQuery - _owner_ to _role_ switch is handled in Query.Builder. All 
internal searching is now handled via _role_.
* JobConfiguration - internal _owner_ refs redirected to _key.role_ 0.7.0.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 
83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c 
  src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 
5c75cc8cae53edfa069c85c37ebad34774682081 
  src/main/java/org/apache/aurora/scheduler/TaskVars.java 
f1ab934541ad6d9ae74927f80a9c654a04922eb5 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
e9f251508257cd7287ff00773e0073a3cd130df8 
  src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
a76c3fac71b35115064fba6644cff0066fd9e630 
  src/main/java/org/apache/aurora/scheduler/base/Query.java 
d518acbff04fecda5c3592340e78fc77da9e339d 
  src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
6ad79270c35c4fccb01f29d34ef1c4bbd7c953c8 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 5871dcad819b27db7379c69c6f8ff69f4a1b0104 
  src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
a0cb7bf56aeb7edd92b25d8d69a739d87452777a 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
5f08997f04ffa7d9610c2b41551943b563412626 
  src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
2d27ad953c5f3763bf23f7e6efc26710f32906b6 
  src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
58b94c2f2f3bac00f0692579974e8bdf159b6e40 
  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
3b3cef2a1b09c85aa06e45e734164f87f2510c55 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
37176237fac336413267f3c8bb4e1b9a6255150c 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
610fe02b96c5d5fc3506aff6bf518220bc1ae0cd 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
6ec130f4a9a5075b34452efb27c8fd0f08f93a63 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
7a4aa73178299651c21e44699ec3e9e494eaa4ab 
  src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java 
e96974764844b5d1a3a05f6996075fccee209594 
  src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
371ae87f5954fa5f092db1f6d21e2291d7576173 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
c405d4c8b127c2dd7054c9520064da0346690f02 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
8ee84cda8670d117e2efa2d1a114da6f0d8315d6 
  src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
b3e4ae39067b1dfb632f5d685d69fcbd7d4705da 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
6534329a92bf005223fa8907cbe4a8a3a511e142 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
919c79e8cec6441a19db2afdd4c414dc6f0e0dc5 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
d2d3e86bb5acf3402f55188b9ae440412ef14b5a 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
33790b118e788d7c894f7635f896619a3266192a 
  src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
21640f7ec2172d4c1b1bc744a4d71a6fa0a29376 
  src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
cf4a015a040338a642fb07eec1fb7b5c11058fe5 
  src/test/java/org/apache/aurora/scheduler/stats/ResourceCounterTest.java 
fc12933cefdbfc03de5918de04172c0744d34588 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
0c1a271972702c64d8a4db3d3d1127a00d48d679 
  src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 
581f6391c59244b3376d2796b2c7cd8650549c3b 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 3fde3f75f8476338af2dde12862af39c8ad67226 
  
src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java
 5242a43b1c8a236b9420625a64ff24ad9ff75643 
  

Re: Review Request 26762: Preparing for Identity struct deprecation (scheduler).

2014-10-22 Thread Bill Farner

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

Ship it!



src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
https://reviews.apache.org/r/26762/#comment98769

Is this safe to remove?  I see red but no corresponding green.



src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
https://reviews.apache.org/r/26762/#comment98774

Does it make sense to reorder this for more code reuse and predictable 
outcome?

- if key is not set, populate it
- validate key _and_ owner



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
https://reviews.apache.org/r/26762/#comment98789

For other reviewers - this is safe because 
`SanitizedConfiguration.fromUnsanitized` requires consistency between `key` and 
`owner` fields, so worst case is that they are inconsistent (and would fail 
sanitization anyway) and you auth against the wrong user.



src/main/thrift/org/apache/aurora/gen/api.thrift
https://reviews.apache.org/r/26762/#comment98763

How about s/key/job/?  It's less ambiguous (since we could conceivably 
introduce a TaskConfigKey), and sufficiently desriptive.


- Bill Farner


On Oct. 22, 2014, 9:06 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26762/
 ---
 
 (Updated Oct. 22, 2014, 9:06 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-84
 https://issues.apache.org/jira/browse/AURORA-84
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This patch replaces the Identity struct use within the scheduler and makes it 
 ready for removal (i.e. when the JobStore is ready). 
 
 Sending it as a separate CR for easier reviewing. 
 
 Will have to be committed along with python changes 
 (https://reviews.apache.org/r/26954/) to avoid breaking client diff 
 functionality.
 
 Summary of the changes:
 * TaskConfig - dual write in StorageBackfill to populate new _key_ field. 
 Incoming thrift objects are populated in ConfigurationManager during 
 sanitizing.
 * TaskQuery - _owner_ to _role_ switch is handled in Query.Builder. All 
 internal searching is now handled via _role_.
 * JobConfiguration - internal _owner_ refs redirected to _key.role_ 0.7.0.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 
 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c 
   src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 
 5c75cc8cae53edfa069c85c37ebad34774682081 
   src/main/java/org/apache/aurora/scheduler/TaskVars.java 
 f1ab934541ad6d9ae74927f80a9c654a04922eb5 
   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
 e9f251508257cd7287ff00773e0073a3cd130df8 
   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
 a76c3fac71b35115064fba6644cff0066fd9e630 
   src/main/java/org/apache/aurora/scheduler/base/Query.java 
 eded7a59eb394748b93d7fbc085a1bdf64b043cc 
   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
 6ad79270c35c4fccb01f29d34ef1c4bbd7c953c8 
   
 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
  865742171c11fbe5cf1469a69dd7258ec1be28c2 
   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
 a0cb7bf56aeb7edd92b25d8d69a739d87452777a 
   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
 5f08997f04ffa7d9610c2b41551943b563412626 
   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
 0f6731106c53420b92e60b9faf26c3614bd7ae00 
   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
 58b94c2f2f3bac00f0692579974e8bdf159b6e40 
   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
 37176237fac336413267f3c8bb4e1b9a6255150c 
   
 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
  137f97d33decd14bf2f6dcdd9cd18c3db2b7c89c 
   
 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
  6ec130f4a9a5075b34452efb27c8fd0f08f93a63 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 8794731f4b3f1033588bdfa33c292e4796319a2a 
   src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java 
 e96974764844b5d1a3a05f6996075fccee209594 
   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
 371ae87f5954fa5f092db1f6d21e2291d7576173 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 606c4434b7158220ccf1403b6deac939021fee31 
   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
 8ee84cda8670d117e2efa2d1a114da6f0d8315d6