Re: Review Request 42656: Don't destroy session between requests with TRequestsTransport

[Stephan Erb]

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



Would be great if you could also run the end-to-end tests to verify nothing is 
broken.


src/main/python/apache/aurora/common/transport.py (line 114)


Is this remaining if necessary at all? `isOpen()`will only return `False` 
if somebody has called `close()`. That should never happen here, as it would 
imply somebody is calling  `flush()` on a closed transport.


- Stephan Erb


On Jan. 22, 2016, 8:29 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42656/
> ---
> 
> (Updated Jan. 22, 2016, 8:29 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> As an API consumer, I want the API client to reuse open connections and not 
> create a new one for every query.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 909021ac203185862267d4359d332fc169a06b7e 
>   src/test/python/apache/aurora/common/test_transport.py 
> 1f589a9ae08e1f13be34ad6002ceb11a43fdeb5f 
> 
> Diff: https://reviews.apache.org/r/42656/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest src/test/python::
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 42668: Remove most direct uses of deprecated TaskConfig fields.

[Zameer Manji]

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


Ship it!




Ship It!

- Zameer Manji


On Jan. 22, 2016, 11:23 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42668/
> ---
> 
> (Updated Jan. 22, 2016, 11:23 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> After https://reviews.apache.org/r/42646/, i ventured towards removing 
> TaskConfig fields that have been marked as deprecated:
> 
> TaskConfig.owner.role
> TaskConfig.environment
> TaskConfig.jobName
> 
> Turns out we still cannot remove them, but i resolved to at least prepare a 
> lot of our code for the removal.  The majority of the work here was 
> converting unit tests to use `TaskTestUtil`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 440946bf6cb9dac21b64acdca3678b505ef37aff 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> 37700569c5b8d14dcc7f29ac564cb6546f73ea34 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
> ebb4f9a20e382360032bfad3389349ef13117460 
>   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
> 0a9dfe31b7680d92e8101abfad4e88a324f37a77 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> f0624976259762cc2c86a653e27e9e5ea1bc71f0 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  3b1766d67e5bfcb5b5d1bd1759d251b7f1732e9d 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 2b5a82db77827013f86880d497cde3758276878a 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 066c6a3393955ca4dc67e5ddfc6ead9b451b9b4e 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> e1b539136fde48b95ab9f00d31b0e5674e27d37d 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 920e3e594330ed87c69c2cbf65607621478685b4 
>   
> src/test/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculatorImplTest.java
>  9d21dcd47995d9955c3b4ecfb1a7c1f863022d81 
>   src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
> 89162d12aee13bd70f340df9ea64eb86e555f8d3 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
> 7fb6278d75561c2d38fd7a92b168563d20e8c1d9 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 4a655f10b96b91e48b465a10569bbbc7a62c412f 
>   
> src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
>  092df8cdb7901e045682d0f252109a08afc50c01 
>   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
> d0a6cd6e48d6d5623b4b7a68f9e6d5336c61bbe7 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java
>  fffbc36d034525e012fd19e95e91fd5bae152c5d 
> 
> Diff: https://reviews.apache.org/r/42668/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Aurora ReviewBot]

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


Ship it!




Master (4b3d7bc) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Jan. 22, 2016, 9:43 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 9:43 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS c9ff0257a9be75b590c4d4829dca49f575bb00e2 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
>  065ad425a0d359c3d73db197b528ab45bf9f6bda 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

> On Jan. 22, 2016, 9:48 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 439
> > 
> >
> > DRY?  I actually meant to only do the `implicitKillQuery` 1x at the end.

`query` has to remain final to be used later, so this was the easiest approach 
without extra assignments.


- Maxim


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


On Jan. 22, 2016, 9:43 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 9:43 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS c9ff0257a9be75b590c4d4829dca49f575bb00e2 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
>  065ad425a0d359c3d73db197b528ab45bf9f6bda 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Aurora ReviewBot]

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


Ship it!




Master (2da1700) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Jan. 22, 2016, 9:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 9:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42668: Remove most direct uses of deprecated TaskConfig fields.

[Maxim Khutornenko]

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


Ship it!




Ship It!

- Maxim Khutornenko


On Jan. 22, 2016, 7:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42668/
> ---
> 
> (Updated Jan. 22, 2016, 7:23 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> After https://reviews.apache.org/r/42646/, i ventured towards removing 
> TaskConfig fields that have been marked as deprecated:
> 
> TaskConfig.owner.role
> TaskConfig.environment
> TaskConfig.jobName
> 
> Turns out we still cannot remove them, but i resolved to at least prepare a 
> lot of our code for the removal.  The majority of the work here was 
> converting unit tests to use `TaskTestUtil`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 440946bf6cb9dac21b64acdca3678b505ef37aff 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> 37700569c5b8d14dcc7f29ac564cb6546f73ea34 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
> ebb4f9a20e382360032bfad3389349ef13117460 
>   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
> 0a9dfe31b7680d92e8101abfad4e88a324f37a77 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> f0624976259762cc2c86a653e27e9e5ea1bc71f0 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  3b1766d67e5bfcb5b5d1bd1759d251b7f1732e9d 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 2b5a82db77827013f86880d497cde3758276878a 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 066c6a3393955ca4dc67e5ddfc6ead9b451b9b4e 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> e1b539136fde48b95ab9f00d31b0e5674e27d37d 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 920e3e594330ed87c69c2cbf65607621478685b4 
>   
> src/test/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculatorImplTest.java
>  9d21dcd47995d9955c3b4ecfb1a7c1f863022d81 
>   src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
> 89162d12aee13bd70f340df9ea64eb86e555f8d3 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
> 7fb6278d75561c2d38fd7a92b168563d20e8c1d9 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 4a655f10b96b91e48b465a10569bbbc7a62c412f 
>   
> src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
>  092df8cdb7901e045682d0f252109a08afc50c01 
>   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
> d0a6cd6e48d6d5623b4b7a68f9e6d5336c61bbe7 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java
>  fffbc36d034525e012fd19e95e91fd5bae152c5d 
> 
> Diff: https://reviews.apache.org/r/42668/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Bill Farner]

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


Ship it!





src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
(line 438)


DRY?  I actually meant to only do the `implicitKillQuery` 1x at the end.


- Bill Farner


On Jan. 22, 2016, 1:43 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 1:43 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS c9ff0257a9be75b590c4d4829dca49f575bb00e2 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
>  065ad425a0d359c3d73db197b528ab45bf9f6bda 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

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

(Updated Jan. 22, 2016, 9:43 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Rebased.


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


Repository: aurora


Description
---

Following the path consistent with restartShards using JobKey + instanceIds.


Diffs (updated)
-

  NEWS c9ff0257a9be75b590c4d4829dca49f575bb00e2 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f099620967a7836538f19545fe373faa335529a4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
69eab90fa053c917a6a2c60b548802ba450fa80c 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
  src/main/python/apache/aurora/client/api/__init__.py 
ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java 
3e811a4f4d2c82892217ca1f950ac792303fbcf3 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 79e70fde1633e6dfbed2171cceee0197acae2550 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d12b56eb675e5187fb375a27ace3849fdab1dd8d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
 065ad425a0d359c3d73db197b528ab45bf9f6bda 
  src/test/python/apache/aurora/api_util.py 
5b2a5383ffa490dcf651c027074ed42bba0ab644 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 

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


Testing
---

./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Aurora ReviewBot]

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



This patch does not apply cleanly against master (2da1700), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Jan. 22, 2016, 9:36 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 9:36 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
>  065ad425a0d359c3d73db197b528ab45bf9f6bda 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Bill Farner]

> On Jan. 22, 2016, 1:19 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 441
> > 
> >
> > Following up from earlier comment - `active()` scoping is absent here, 
> > which can result in log noise as illegal task transitions are attempted.
> > 
> > Consider pulling the `implicitKillQuery` call out so that it covers all 
> > branches.
> 
> Maxim Khutornenko wrote:
> Misread your comment, I see what you meant now! Refactored implicitQuery 
> to account for both.

Oh you read the comment correctly, i read the code incorrectly :-)


- Bill


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


On Jan. 22, 2016, 1:13 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 1:13 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
>  065ad425a0d359c3d73db197b528ab45bf9f6bda 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

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

(Updated Jan. 22, 2016, 9:36 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Setting active for all of job-scoped path.


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


Repository: aurora


Description
---

Following the path consistent with restartShards using JobKey + instanceIds.


Diffs (updated)
-

  NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f099620967a7836538f19545fe373faa335529a4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
69eab90fa053c917a6a2c60b548802ba450fa80c 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
  src/main/python/apache/aurora/client/api/__init__.py 
ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java 
3e811a4f4d2c82892217ca1f950ac792303fbcf3 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 79e70fde1633e6dfbed2171cceee0197acae2550 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d12b56eb675e5187fb375a27ace3849fdab1dd8d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
 065ad425a0d359c3d73db197b528ab45bf9f6bda 
  src/test/python/apache/aurora/api_util.py 
5b2a5383ffa490dcf651c027074ed42bba0ab644 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 

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


Testing
---

./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

> On Jan. 22, 2016, 9:19 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 441
> > 
> >
> > Following up from earlier comment - `active()` scoping is absent here, 
> > which can result in log noise as illegal task transitions are attempted.
> > 
> > Consider pulling the `implicitKillQuery` call out so that it covers all 
> > branches.

Misread your comment, I see what you meant now! Refactored implicitQuery to 
account for both.


- Maxim


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


On Jan. 22, 2016, 9:13 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 9:13 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
>  065ad425a0d359c3d73db197b528ab45bf9f6bda 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Maxim Khutornenko]

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


Ship it!




Ship It!

- Maxim Khutornenko


On Jan. 22, 2016, 9:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 9:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Bill Farner]

> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.
> 
> Bill Farner wrote:
> That is essentially the stance i intend.  My hope is that when we _do_ 
> need it again, the resulting approach has less code ripple than we see here.  
> I could be deluded, however.
> 
> John Sirois wrote:
> You may be, I'm not sure.  Happy to accept that intentioned stance though.
> 
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I 
> personally remember reviving it once. Not really sure what killing again and 
> again really gets us. Can we finally accept it's existence and keep it around 
> as a placeholder? 
> 
> When we _do_ need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but 
> remove StorageBackfill?  That will make it easy to wire in the next backfill 
> function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Call `mutateTask()` for every task _changed_.
> 
> Maxim Khutornenko wrote:
> I am still not sure why having a placeholder NOOP class is such a bad 
> idea. Properly documented, it may serve as a great ramp up to whoever 
> attempts to backfill in the future. 
> 
> If you despise having NOOP classes how about converting it into an 
> interface instead? That may be a great place to put all guidance details.
> 
> Bill Farner wrote:
> But it will literally be an empty class with an empty function.  I don't 
> see that as much of a ramp.
> 
> Maxim Khutornenko wrote:
> Well, I tried... Let's at least keep the initialization point as you 
> suggested.

Not trying to be argumentative here, hope it didn't come across this way.  
Here's what would be left of `StorageBackfill`:

```
public final class StorageBackfill {

  private StorageBackfill() {
// Utility class.
  }

  public static void backfill(final MutableStoreProvider storeProvider) {
  }
}
```


- Bill


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


On Jan. 22, 2016, 1:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 1:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/j

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[John Sirois]

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


Ship it!




Ship It!

- John Sirois


On Jan. 22, 2016, 2:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 2:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Bill Farner]

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

(Updated Jan. 22, 2016, 1:21 p.m.)


Review request for Aurora, John Sirois and Maxim Khutornenko.


Changes
---

Restored initialization logic when starting storage.


Repository: aurora


Description
---

Related to a comment i made in https://reviews.apache.org/r/42628/, 
StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
you can see from the patch, there's a lot that crumbles with it.

Since it doesn't show in the patch, StorageBackfill was only filling the 
`TaskConfig.job` field in tasks and cron jobs:
https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72

This backfill has been in place since `0.6.0-incubating` and should be safe to 
remove.
https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
0d3874e9632952c54ef5ae9aba78638e47c87056 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
376f48545eb860aad5afa028d3b96d04acc08377 
  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
  src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
4e4f8d2c0f237c4480abe101835176f7d69958db 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
728353116c8892c015a6443d8db09ba241b32230 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
8fd024a460cd948dab703b99788b1ed2d6c43bc3 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
9fb8aad5d1c0412efc6d1176e543321ebe503e03 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
1ac41d18fd9d603c4c342f9635e116bfd055f858 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
2570464c85630a55d3e6c8653fc0307d54504e15 
  
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
 3c4e2bd5eefc141a5d2c5d0e56881899816652f8 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Bill Farner]

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


Fix it, then Ship it!





src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
(line 441)


Following up from earlier comment - `active()` scoping is absent here, 
which can result in log noise as illegal task transitions are attempted.

Consider pulling the `implicitKillQuery` call out so that it covers all 
branches.


- Bill Farner


On Jan. 22, 2016, 1:13 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 1:13 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
>  065ad425a0d359c3d73db197b528ab45bf9f6bda 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

> On Jan. 22, 2016, 7:44 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 444
> > 
> >
> > Nearly forgot - in this branch, you should log and return a deprecation 
> > warning.  Cmd-f for 'deprecated' in this file and you will spot an example.

Done.


- Maxim


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


On Jan. 22, 2016, 6:47 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 6:47 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

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

(Updated Jan. 22, 2016, 9:13 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Bill's comments and test fixes.


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


Repository: aurora


Description
---

Following the path consistent with restartShards using JobKey + instanceIds.


Diffs (updated)
-

  NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f099620967a7836538f19545fe373faa335529a4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
69eab90fa053c917a6a2c60b548802ba450fa80c 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
  src/main/python/apache/aurora/client/api/__init__.py 
ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java 
3e811a4f4d2c82892217ca1f950ac792303fbcf3 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 79e70fde1633e6dfbed2171cceee0197acae2550 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d12b56eb675e5187fb375a27ace3849fdab1dd8d 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdminTest.java
 065ad425a0d359c3d73db197b528ab45bf9f6bda 
  src/test/python/apache/aurora/api_util.py 
5b2a5383ffa490dcf651c027074ed42bba0ab644 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 

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


Testing
---

./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

> On Jan. 22, 2016, 7:42 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java,
> >  line 132
> > 
> >
> > s/consider removing/Remove/, + link ticket.  When we were first 
> > building this code, the wildcard was a real pain.  Removing it will be 
> > welcome!

Done.


> On Jan. 22, 2016, 7:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 438
> > 
> >
> > s/.active()//, that's already done by `implicitKillQuery()`.

implicitKillQuery is not not called in this code path.


> On Jan. 22, 2016, 7:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 429
> > 
> >
> > As you've done in `AnnotatedAuroraAdmin`, all 4 of these params should 
> > be marked `@Nullable`.

Done.


- Maxim


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


On Jan. 22, 2016, 6:47 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 6:47 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Maxim Khutornenko]

> On Jan. 22, 2016, 6:28 p.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.
> 
> Bill Farner wrote:
> That is essentially the stance i intend.  My hope is that when we _do_ 
> need it again, the resulting approach has less code ripple than we see here.  
> I could be deluded, however.
> 
> John Sirois wrote:
> You may be, I'm not sure.  Happy to accept that intentioned stance though.
> 
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I 
> personally remember reviving it once. Not really sure what killing again and 
> again really gets us. Can we finally accept it's existence and keep it around 
> as a placeholder? 
> 
> When we _do_ need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but 
> remove StorageBackfill?  That will make it easy to wire in the next backfill 
> function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Call `mutateTask()` for every task _changed_.
> 
> Maxim Khutornenko wrote:
> I am still not sure why having a placeholder NOOP class is such a bad 
> idea. Properly documented, it may serve as a great ramp up to whoever 
> attempts to backfill in the future. 
> 
> If you despise having NOOP classes how about converting it into an 
> interface instead? That may be a great place to put all guidance details.
> 
> Bill Farner wrote:
> But it will literally be an empty class with an empty function.  I don't 
> see that as much of a ramp.

Well, I tried... Let's at least keep the initialization point as you suggested.


- Maxim


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


On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 8:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Bill Farner]

> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.
> 
> Bill Farner wrote:
> That is essentially the stance i intend.  My hope is that when we _do_ 
> need it again, the resulting approach has less code ripple than we see here.  
> I could be deluded, however.
> 
> John Sirois wrote:
> You may be, I'm not sure.  Happy to accept that intentioned stance though.
> 
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I 
> personally remember reviving it once. Not really sure what killing again and 
> again really gets us. Can we finally accept it's existence and keep it around 
> as a placeholder? 
> 
> When we _do_ need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but 
> remove StorageBackfill?  That will make it easy to wire in the next backfill 
> function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Call `mutateTask()` for every task _changed_.
> 
> Maxim Khutornenko wrote:
> I am still not sure why having a placeholder NOOP class is such a bad 
> idea. Properly documented, it may serve as a great ramp up to whoever 
> attempts to backfill in the future. 
> 
> If you despise having NOOP classes how about converting it into an 
> interface instead? That may be a great place to put all guidance details.

But it will literally be an empty class with an empty function.  I don't see 
that as much of a ramp.


- Bill


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


On Jan. 22, 2016, 12:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 12:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321eb

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Maxim Khutornenko]

> On Jan. 22, 2016, 6:28 p.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.
> 
> Bill Farner wrote:
> That is essentially the stance i intend.  My hope is that when we _do_ 
> need it again, the resulting approach has less code ripple than we see here.  
> I could be deluded, however.
> 
> John Sirois wrote:
> You may be, I'm not sure.  Happy to accept that intentioned stance though.
> 
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I 
> personally remember reviving it once. Not really sure what killing again and 
> again really gets us. Can we finally accept it's existence and keep it around 
> as a placeholder? 
> 
> When we _do_ need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Bill Farner wrote:
> How about we keep the plumbing for the initialization operation, but 
> remove StorageBackfill?  That will make it easy to wire in the next backfill 
> function, but doesn't leave us with a dead class.
> > When we do need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?
> 
> Call `mutateTask()` for every task _changed_.

I am still not sure why having a placeholder NOOP class is such a bad idea. 
Properly documented, it may serve as a great ramp up to whoever attempts to 
backfill in the future. 

If you despise having NOOP classes how about converting it into an interface 
instead? That may be a great place to put all guidance details.


- Maxim


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


On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 8:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/j

Re: Review Request 42639: Simplify TaskHistoryPruner tie-in to Lifecycle.

[Bill Farner]

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

Ship it!


Ship It!

- Bill Farner


On Jan. 21, 2016, 6:47 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42639/
> ---
> 
> (Updated Jan. 21, 2016, 6:47 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1582
> https://issues.apache.org/jira/browse/AURORA-1582
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This eliminates processing all futures to find the 1st failed one in
> favor of directly signalling a Service failure when a unit of async work
> fails.
> 
>  src/main/java/org/apache/aurora/GuavaUtils.java  | 
> 18 
>  src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java | 
> 56 ++
>  src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java   | 
> 17 ++-
>  3 files changed, 40 insertions(+), 51 deletions(-)
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 8c2ab5720b808e5eae4c6b93fea8b787839b8dbe 
>   src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 
> 2d4c58eaa930c561446320a77a559cd926318e8a 
>   src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java 
> 8d19c04a96fccdd9594ecf783fc797c2b76140c7 
> 
> Diff: https://reviews.apache.org/r/42639/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./gradlew -P build`.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Bill Farner]

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



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
(line 444)


Nearly forgot - in this branch, you should log and return a deprecation 
warning.  Cmd-f for 'deprecated' in this file and you will spot an example.


- Bill Farner


On Jan. 22, 2016, 10:47 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 10:47 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Bill Farner]

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

Ship it!



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
(line 429)


As you've done in `AnnotatedAuroraAdmin`, all 4 of these params should be 
marked `@Nullable`.



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
(line 438)


s/.active()//, that's already done by `implicitKillQuery()`.



src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 (line 130)


s/consider removing/Remove/, + link ticket.  When we were first building 
this code, the wildcard was a real pain.  Removing it will be welcome!


- Bill Farner


On Jan. 22, 2016, 10:47 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 10:47 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42668: Remove most direct uses of deprecated TaskConfig fields.

[Aurora ReviewBot]

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

Ship it!


Master (66a4d5f) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Jan. 22, 2016, 7:23 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42668/
> ---
> 
> (Updated Jan. 22, 2016, 7:23 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> After https://reviews.apache.org/r/42646/, i ventured towards removing 
> TaskConfig fields that have been marked as deprecated:
> 
> TaskConfig.owner.role
> TaskConfig.environment
> TaskConfig.jobName
> 
> Turns out we still cannot remove them, but i resolved to at least prepare a 
> lot of our code for the removal.  The majority of the work here was 
> converting unit tests to use `TaskTestUtil`.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
> 440946bf6cb9dac21b64acdca3678b505ef37aff 
>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
> 37700569c5b8d14dcc7f29ac564cb6546f73ea34 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
> ebb4f9a20e382360032bfad3389349ef13117460 
>   src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
> 0a9dfe31b7680d92e8101abfad4e88a324f37a77 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> f0624976259762cc2c86a653e27e9e5ea1bc71f0 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  3b1766d67e5bfcb5b5d1bd1759d251b7f1732e9d 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 2b5a82db77827013f86880d497cde3758276878a 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 066c6a3393955ca4dc67e5ddfc6ead9b451b9b4e 
>   
> src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
> e1b539136fde48b95ab9f00d31b0e5674e27d37d 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 920e3e594330ed87c69c2cbf65607621478685b4 
>   
> src/test/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculatorImplTest.java
>  9d21dcd47995d9955c3b4ecfb1a7c1f863022d81 
>   src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
> 89162d12aee13bd70f340df9ea64eb86e555f8d3 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
> 7fb6278d75561c2d38fd7a92b168563d20e8c1d9 
>   src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
> 4a655f10b96b91e48b465a10569bbbc7a62c412f 
>   
> src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
>  092df8cdb7901e045682d0f252109a08afc50c01 
>   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
> d0a6cd6e48d6d5623b4b7a68f9e6d5336c61bbe7 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java
>  fffbc36d034525e012fd19e95e91fd5bae152c5d 
> 
> Diff: https://reviews.apache.org/r/42668/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Bill Farner]

> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.
> 
> Bill Farner wrote:
> That is essentially the stance i intend.  My hope is that when we _do_ 
> need it again, the resulting approach has less code ripple than we see here.  
> I could be deluded, however.
> 
> John Sirois wrote:
> You may be, I'm not sure.  Happy to accept that intentioned stance though.
> 
> Maxim Khutornenko wrote:
> We have killed and resurrected StorageBackfill a few times already. I 
> personally remember reviving it once. Not really sure what killing again and 
> again really gets us. Can we finally accept it's existence and keep it around 
> as a placeholder? 
> 
> When we _do_ need it next time, what's the storage support story for it? 
> Call mutateTask() for every task processed?

How about we keep the plumbing for the initialization operation, but remove 
StorageBackfill?  That will make it easy to wire in the next backfill function, 
but doesn't leave us with a dead class.
> When we do need it next time, what's the storage support story for it? Call 
> mutateTask() for every task processed?

Call `mutateTask()` for every task _changed_.


- Bill


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


On Jan. 22, 2016, 12:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 12:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/te

Review Request 42668: Remove most direct uses of deprecated TaskConfig fields.

[Bill Farner]

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

Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Repository: aurora


Description
---

After https://reviews.apache.org/r/42646/, i ventured towards removing 
TaskConfig fields that have been marked as deprecated:

TaskConfig.owner.role
TaskConfig.environment
TaskConfig.jobName

Turns out we still cannot remove them, but i resolved to at least prepare a lot 
of our code for the removal.  The majority of the work here was converting unit 
tests to use `TaskTestUtil`.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/Utilization.java 
440946bf6cb9dac21b64acdca3678b505ef37aff 
  src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 
37700569c5b8d14dcc7f29ac564cb6546f73ea34 
  src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
4e4f8d2c0f237c4480abe101835176f7d69958db 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
69eab90fa053c917a6a2c60b548802ba450fa80c 
  src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java 
ebb4f9a20e382360032bfad3389349ef13117460 
  src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
0a9dfe31b7680d92e8101abfad4e88a324f37a77 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
9fb8aad5d1c0412efc6d1176e543321ebe503e03 
  src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
f0624976259762cc2c86a653e27e9e5ea1bc71f0 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
3b1766d67e5bfcb5b5d1bd1759d251b7f1732e9d 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
2b5a82db77827013f86880d497cde3758276878a 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
066c6a3393955ca4dc67e5ddfc6ead9b451b9b4e 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 
e1b539136fde48b95ab9f00d31b0e5674e27d37d 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
920e3e594330ed87c69c2cbf65607621478685b4 
  
src/test/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculatorImplTest.java
 9d21dcd47995d9955c3b4ecfb1a7c1f863022d81 
  src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java 
89162d12aee13bd70f340df9ea64eb86e555f8d3 
  src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 
7fb6278d75561c2d38fd7a92b168563d20e8c1d9 
  src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java 
4a655f10b96b91e48b465a10569bbbc7a62c412f 
  
src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
 092df8cdb7901e045682d0f252109a08afc50c01 
  src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
d0a6cd6e48d6d5623b4b7a68f9e6d5336c61bbe7 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d12b56eb675e5187fb375a27ace3849fdab1dd8d 
  
src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java
 fffbc36d034525e012fd19e95e91fd5bae152c5d 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Aurora ReviewBot]

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


Master (66a4d5f) is red with this patch.
  ./build-support/jenkins/build.sh

:findbugsJmh
:findbugsMain
:findbugsTest
:licenseJmh UP-TO-DATE
:licenseMain UP-TO-DATE
:licenseTest UP-TO-DATE
:license UP-TO-DATE
:pmdJmh
:pmdMain
:pmdTest
:test

org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdminTest > 
testAllAuroraSchedulerManagerIfaceMethodsHaveAuthorizingParam FAILED
java.lang.AssertionError at AnnotatedAuroraAdminTest.java:48

org.apache.aurora.scheduler.http.api.security.HttpSecurityIT > 
testAuroraSchedulerManager FAILED
org.apache.thrift.transport.TTransportException at HttpSecurityIT.java:293
java.lang.AssertionError
I0122 19:14:56.712 [ShutdownHook, SchedulerMain:104] Stopping scheduler 
services. 

931 tests completed, 2 failed, 2 skipped
:test FAILED
:jacocoTestReport
Coverage report generated: 
file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/jacoco/test/html/index.html
:analyzeReport
Instruction coverage is 0.8882782996029824, but must be greater than 0.89
Branch coverage is 0.8200636942675159, but must be greater than 0.835

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/tests/index.html

* 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: 4 mins 39.951 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Jan. 22, 2016, 6:47 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 6:47 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42666: Deprecating TaskQuery in killTasks.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 22, 2016, 10:47 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42666/
> ---
> 
> (Updated Jan. 22, 2016, 10:47 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1583
> https://issues.apache.org/jira/browse/AURORA-1583
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Following the path consistent with restartShards using JobKey + instanceIds.
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f099620967a7836538f19545fe373faa335529a4 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  69eab90fa053c917a6a2c60b548802ba450fa80c 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java
>  f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
>   src/main/python/apache/aurora/client/api/__init__.py 
> ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java
>  3e811a4f4d2c82892217ca1f950ac792303fbcf3 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
>  79e70fde1633e6dfbed2171cceee0197acae2550 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  d12b56eb675e5187fb375a27ace3849fdab1dd8d 
>   src/test/python/apache/aurora/api_util.py 
> 5b2a5383ffa490dcf651c027074ed42bba0ab644 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 
> 
> Diff: https://reviews.apache.org/r/42666/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 42645: Remove scheduler flag -extra_modules.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Jan. 21, 2016, 11:02 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42645/
> ---
> 
> (Updated Jan. 21, 2016, 11:02 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Split out of https://reviews.apache.org/r/42565/
> 
> 
> Diffs
> -
> 
>   NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0659c358479283756179c2cabebc8416730cc3e3 
> 
> Diff: https://reviews.apache.org/r/42645/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[John Sirois]

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

Ship it!


Ship It!

- John Sirois


On Jan. 22, 2016, 1:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 1:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 42666: Deprecating TaskQuery in killTasks.

[Maxim Khutornenko]

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

Review request for Aurora, Bill Farner and Zameer Manji.


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


Repository: aurora


Description
---

Following the path consistent with restartShards using JobKey + instanceIds.


Diffs
-

  NEWS 37a7a7933b20e112416b01260d97fc2f47c24027 
  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f099620967a7836538f19545fe373faa335529a4 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
69eab90fa053c917a6a2c60b548802ba450fa80c 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 
f6669efe9d00ead3f123dda6f6152ae273e1a6c4 
  src/main/python/apache/aurora/client/api/__init__.py 
ac4e6f209e4bd8a9923c6cff17b1ef1486bf3ab6 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java 
3e811a4f4d2c82892217ca1f950ac792303fbcf3 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java
 79e70fde1633e6dfbed2171cceee0197acae2550 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 d12b56eb675e5187fb375a27ace3849fdab1dd8d 
  src/test/python/apache/aurora/api_util.py 
5b2a5383ffa490dcf651c027074ed42bba0ab644 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
2daa96c4725ca24b8bd17f9dbdbbdc463b0facf8 

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


Testing
---

./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Maxim Khutornenko]

> On Jan. 22, 2016, 6:28 p.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.
> 
> Bill Farner wrote:
> That is essentially the stance i intend.  My hope is that when we _do_ 
> need it again, the resulting approach has less code ripple than we see here.  
> I could be deluded, however.
> 
> John Sirois wrote:
> You may be, I'm not sure.  Happy to accept that intentioned stance though.

We have killed and resurrected StorageBackfill a few times already. I 
personally remember reviving it once. Not really sure what killing again and 
again really gets us. Can we finally accept it's existence and keep it around 
as a placeholder? 

When we _do_ need it next time, what's the storage support story for it? Call 
mutateTask() for every task processed?


- Maxim


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


On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 8:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[John Sirois]

> On Jan. 22, 2016, 11:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.
> 
> Bill Farner wrote:
> That is essentially the stance i intend.  My hope is that when we _do_ 
> need it again, the resulting approach has less code ripple than we see here.  
> I could be deluded, however.

You may be, I'm not sure.  Happy to accept that intentioned stance though.


- John


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


On Jan. 22, 2016, 1:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 1:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Bill Farner]

> On Jan. 22, 2016, 10:28 a.m., John Sirois wrote:
> > The mechanics of the change lgtm, but is there some other way to add schema 
> > / data updates to a new aurora release such that they get applied after 
> > recovering the log but before doing anything else?  I guess its valid to 
> > say add it back when we need it, but I want to make sure this is the stance 
> > you intend; ie remove the burden from the existing codebase and push it to 
> > the next contributor that needs to break the schema.

That is essentially the stance i intend.  My hope is that when we _do_ need it 
again, the resulting approach has less code ripple than we see here.  I could 
be deluded, however.


- Bill


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


On Jan. 22, 2016, 12:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 12:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[John Sirois]

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


The mechanics of the change lgtm, but is there some other way to add schema / 
data updates to a new aurora release such that they get applied after 
recovering the log but before doing anything else?  I guess its valid to say 
add it back when we need it, but I want to make sure this is the stance you 
intend; ie remove the burden from the existing codebase and push it to the next 
contributor that needs to break the schema.

- John Sirois


On Jan. 22, 2016, 1:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 1:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42565: Remove support for adding guice modules via command line arguments.

[John Sirois]

> On Jan. 21, 2016, 7:44 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > 
> >
> > Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> > 
> > This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> > 
> > I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
> In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
> 
> ```
> public class CustomMain {
>   public static void main(String[] args) {
> SchedulerMain.applyStaticArgumentValues(args);
> SchedulerMain.publicMain(new CustomRealmModule());
>   }
> }
> ```
> 
> (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
> 
> I see 2 upsides with this:
> - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
> - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
> 
> I believe your suggestion to use an INI section falls apart in 2 ways:
> - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
> - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
> 
> I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
> 1. support for participation in guice injection
> 2. support for participation in the same configuration system as the rest 
> of the scheduler
> 
> If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).
> 
> Maxim Khutornenko wrote:
> Perhaps a script example will help evaluating this approach? Bill, would 
> you mind giving a simple example of how aurora-scheduler.conf looked had we 
> added a custom module in our e2e tests?
> 
> Bill Farner wrote:
> `aurora-scheduler.conf` would not change, as you would only alter the 
> main class.  The change would be in `build.gradle`:
> 
> From
> ```
> mainClassName = 'org.apache.aurora.scheduler.app.SchedulerMain'
> ```
> 
> to
> ```
> mainClassName = 'com.my.package.CustomScheduler'
> ```
> 
> Maxim Khutornenko wrote:
> This means anyone who wants to add custom modules would be forced to fork 
> Aurora codebase instead of a purely pluggable model that we have now? I am 
> not sure I like it. This is error and merge conflict prone.
> 
> Perhaps having a well documented gradle option taking a path to 
> CustomScheduler would be a better middle ground? At least that would not 
> require forking Aurora.
> 
> John Sirois wrote:
> Wait - how do you extend aurora today?  My understanding is you need to 
> muck with start scripts to ammend the classpath.  Under that assumption, I 
> saw mucking with classpath + changing main class name as not an undue burden 
> over and above the status quo.
> 
> John Sirois wrote:
> Basically - if you actually want to claim the label pluggable, you need 
> an out-of-packaging way to ammend the classpath.  IE: an optional file in - 
> say ubuntu - /etc/default/aurora.d/my_plugins.conf - that adds a, say 
> 'my_extensions/*' entry to the claaspath.  The need to write a main could be 
> converted to using a requirement to implement a ServiceLoader interface that 
> would allow the standard scheduler main to scan for service implementations - 
> that service might just be a SAM that returns an Iterable of Modules or else 
> it could be a more rigid service interface with 1 method (defaulted) per new 
> extension point that comes up.
> 
> I think this sort of puggability would be great to have, but it seems to 
> me that should be decoupled from the new args system if this change is 
> good-enough, ie:

Re: Review Request 42565: Remove support for adding guice modules via command line arguments.

[John Sirois]

> On Jan. 21, 2016, 7:44 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > 
> >
> > Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> > 
> > This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> > 
> > I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
> In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
> 
> ```
> public class CustomMain {
>   public static void main(String[] args) {
> SchedulerMain.applyStaticArgumentValues(args);
> SchedulerMain.publicMain(new CustomRealmModule());
>   }
> }
> ```
> 
> (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
> 
> I see 2 upsides with this:
> - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
> - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
> 
> I believe your suggestion to use an INI section falls apart in 2 ways:
> - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
> - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
> 
> I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
> 1. support for participation in guice injection
> 2. support for participation in the same configuration system as the rest 
> of the scheduler
> 
> If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).
> 
> Maxim Khutornenko wrote:
> Perhaps a script example will help evaluating this approach? Bill, would 
> you mind giving a simple example of how aurora-scheduler.conf looked had we 
> added a custom module in our e2e tests?
> 
> Bill Farner wrote:
> `aurora-scheduler.conf` would not change, as you would only alter the 
> main class.  The change would be in `build.gradle`:
> 
> From
> ```
> mainClassName = 'org.apache.aurora.scheduler.app.SchedulerMain'
> ```
> 
> to
> ```
> mainClassName = 'com.my.package.CustomScheduler'
> ```
> 
> Maxim Khutornenko wrote:
> This means anyone who wants to add custom modules would be forced to fork 
> Aurora codebase instead of a purely pluggable model that we have now? I am 
> not sure I like it. This is error and merge conflict prone.
> 
> Perhaps having a well documented gradle option taking a path to 
> CustomScheduler would be a better middle ground? At least that would not 
> require forking Aurora.
> 
> John Sirois wrote:
> Wait - how do you extend aurora today?  My understanding is you need to 
> muck with start scripts to ammend the classpath.  Under that assumption, I 
> saw mucking with classpath + changing main class name as not an undue burden 
> over and above the status quo.

Basically - if you actually want to claim the label pluggable, you need an 
out-of-packaging way to ammend the classpath.  IE: an optional file in - say 
ubuntu - /etc/default/aurora.d/my_plugins.conf - that adds a, say 
'my_extensions/*' entry to the claaspath.  The need to write a main could be 
converted to using a requirement to implement a ServiceLoader interface that 
would allow the standard scheduler main to scan for service implementations - 
that service might just be a SAM that returns an Iterable of Modules or else it 
could be a more rigid service interface with 1 method (defaulted) per new 
extension point that comes up.

I think this sort of puggability would be great to have, but it seems to me 
that should be decoupled from the new args system if this change is 
good-enough, ie: maintains the status quo of having to 1.) compile your own 

Re: Review Request 42565: Remove support for adding guice modules via command line arguments.

[John Sirois]

> On Jan. 21, 2016, 7:44 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > 
> >
> > Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> > 
> > This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> > 
> > I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
> In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
> 
> ```
> public class CustomMain {
>   public static void main(String[] args) {
> SchedulerMain.applyStaticArgumentValues(args);
> SchedulerMain.publicMain(new CustomRealmModule());
>   }
> }
> ```
> 
> (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
> 
> I see 2 upsides with this:
> - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
> - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
> 
> I believe your suggestion to use an INI section falls apart in 2 ways:
> - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
> - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
> 
> I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
> 1. support for participation in guice injection
> 2. support for participation in the same configuration system as the rest 
> of the scheduler
> 
> If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).
> 
> Maxim Khutornenko wrote:
> Perhaps a script example will help evaluating this approach? Bill, would 
> you mind giving a simple example of how aurora-scheduler.conf looked had we 
> added a custom module in our e2e tests?
> 
> Bill Farner wrote:
> `aurora-scheduler.conf` would not change, as you would only alter the 
> main class.  The change would be in `build.gradle`:
> 
> From
> ```
> mainClassName = 'org.apache.aurora.scheduler.app.SchedulerMain'
> ```
> 
> to
> ```
> mainClassName = 'com.my.package.CustomScheduler'
> ```
> 
> Maxim Khutornenko wrote:
> This means anyone who wants to add custom modules would be forced to fork 
> Aurora codebase instead of a purely pluggable model that we have now? I am 
> not sure I like it. This is error and merge conflict prone.
> 
> Perhaps having a well documented gradle option taking a path to 
> CustomScheduler would be a better middle ground? At least that would not 
> require forking Aurora.

Wait - how do you extend aurora today?  My understanding is you need to muck 
with start scripts to ammend the classpath.  Under that assumption, I saw 
mucking with classpath + changing main class name as not an undue burden over 
and above the status quo.


- John


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


On Jan. 20, 2016, 1:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42565/
> ---
> 
> (Updated Jan. 20, 2016, 1:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch proposes that users installing custom modules do so via a custom 
> main.
> 
> Specifying custom modules on the command line has proven troublesome for 
> replacing the command line args system with one where

Re: Review Request 42565: Remove support for adding guice modules via command line arguments.

[Maxim Khutornenko]

> On Jan. 22, 2016, 2:44 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > 
> >
> > Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> > 
> > This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> > 
> > I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
> In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
> 
> ```
> public class CustomMain {
>   public static void main(String[] args) {
> SchedulerMain.applyStaticArgumentValues(args);
> SchedulerMain.publicMain(new CustomRealmModule());
>   }
> }
> ```
> 
> (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
> 
> I see 2 upsides with this:
> - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
> - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
> 
> I believe your suggestion to use an INI section falls apart in 2 ways:
> - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
> - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
> 
> I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
> 1. support for participation in guice injection
> 2. support for participation in the same configuration system as the rest 
> of the scheduler
> 
> If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).
> 
> Maxim Khutornenko wrote:
> Perhaps a script example will help evaluating this approach? Bill, would 
> you mind giving a simple example of how aurora-scheduler.conf looked had we 
> added a custom module in our e2e tests?
> 
> Bill Farner wrote:
> `aurora-scheduler.conf` would not change, as you would only alter the 
> main class.  The change would be in `build.gradle`:
> 
> From
> ```
> mainClassName = 'org.apache.aurora.scheduler.app.SchedulerMain'
> ```
> 
> to
> ```
> mainClassName = 'com.my.package.CustomScheduler'
> ```

This means anyone who wants to add custom modules would be forced to fork 
Aurora codebase instead of a purely pluggable model that we have now? I am not 
sure I like it. This is error and merge conflict prone.

Perhaps having a well documented gradle option taking a path to CustomScheduler 
would be a better middle ground? At least that would not require forking Aurora.


- Maxim


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


On Jan. 20, 2016, 8:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42565/
> ---
> 
> (Updated Jan. 20, 2016, 8:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch proposes that users installing custom modules do so via a custom 
> main.
> 
> Specifying custom modules on the command line has proven troublesome for 
> replacing the command line args system with one where all arguments are 
> injected and explicitly-defined.  It also adds complexity to the scheduler 
> application by unnecessarily punching holes at specific places.
> 
> If this proposal is agreeable, i will add a NEWS entry and document how one 
> might implement a custom main to add modules.  The tl;dr, however,

Re: Review Request 42565: Remove support for adding guice modules via command line arguments.

[Bill Farner]

> On Jan. 21, 2016, 6:44 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > 
> >
> > Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> > 
> > This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> > 
> > I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
> In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
> 
> ```
> public class CustomMain {
>   public static void main(String[] args) {
> SchedulerMain.applyStaticArgumentValues(args);
> SchedulerMain.publicMain(new CustomRealmModule());
>   }
> }
> ```
> 
> (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
> 
> I see 2 upsides with this:
> - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
> - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
> 
> I believe your suggestion to use an INI section falls apart in 2 ways:
> - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
> - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
> 
> I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
> 1. support for participation in guice injection
> 2. support for participation in the same configuration system as the rest 
> of the scheduler
> 
> If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).
> 
> Maxim Khutornenko wrote:
> Perhaps a script example will help evaluating this approach? Bill, would 
> you mind giving a simple example of how aurora-scheduler.conf looked had we 
> added a custom module in our e2e tests?

`aurora-scheduler.conf` would not change, as you would only alter the main 
class.  The change would be in `build.gradle`:

From
```
mainClassName = 'org.apache.aurora.scheduler.app.SchedulerMain'
```

to
```
mainClassName = 'com.my.package.CustomScheduler'
```


- Bill


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


On Jan. 20, 2016, 12:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42565/
> ---
> 
> (Updated Jan. 20, 2016, 12:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch proposes that users installing custom modules do so via a custom 
> main.
> 
> Specifying custom modules on the command line has proven troublesome for 
> replacing the command line args system with one where all arguments are 
> injected and explicitly-defined.  It also adds complexity to the scheduler 
> application by unnecessarily punching holes at specific places.
> 
> If this proposal is agreeable, i will add a NEWS entry and document how one 
> might implement a custom main to add modules.  The tl;dr, however, is to 
> invoke `SchedulerMain.publicMain(customModule)`
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/MoreModules.java 
> d5f96543d1068bf30b9d173a247c2806faf35578 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0659c358479283756179c2cabebc8416730cc3e3 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   
> src/main/java

Re: Review Request 42565: Remove support for adding guice modules via command line arguments.

[Maxim Khutornenko]

> On Jan. 22, 2016, 2:44 a.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java,
> >  line 80
> > 
> >
> > Just to be clear, this command line argument will only accept two 
> > modules, the ini realm module and the kerberos realm module?
> > 
> > This means users who need to setup a custom realm (probably any medium 
> > to large cluster operator), must create a custom main.
> > 
> > I'm unsure if this is the ideal approach to take w.r.t security. I 
> > think this requires some discussion on the mailing list. For example, an 
> > alternative to what you have done here would be to remove this argument 
> > entirely, and request users specify realms in the ini file (see 
> > http://shiro.apache.org/configuration.html section "INI Sections").
> 
> Bill Farner wrote:
> In case the concern about a custom main is that it sounds dautning, 
> here's an illustration:
> 
> ```
> public class CustomMain {
>   public static void main(String[] args) {
> SchedulerMain.applyStaticArgumentValues(args);
> SchedulerMain.publicMain(new CustomRealmModule());
>   }
> }
> ```
> 
> (It may be worth building in the `applyStaticArgumentValues` step, so we 
> can reduce even further.)
> 
> I see 2 upsides with this:
> - The API for customization (whether security or other arbitrary feature 
> addition) is uniform.
> - The `CustomMain` can use the same commane line arguments system as the 
> rest of the scheduler (this will work with the forthcoming replacement as 
> well).
> 
> I believe your suggestion to use an INI section falls apart in 2 ways:
> - the `[main]` section only applies when using pure INI configuration for 
> shiro. I believe we fall under the programmatic configuration section, with 
> shiro-guice doing the programmatic bits.  The ini file aurora accepts is read 
> by `org.apache.shiro.realm.text.IniRealm` and it only uses the `users` and 
> `roles` sections (see 
> https://shiro.apache.org/static/1.2.3/apidocs/org/apache/shiro/realm/text/IniRealm.html)
> - i don't believe classes configured with pure shiro INI configuration 
> would participate in guice injection
> 
> I'm open to ideas here.  Ultimately i'm trying to preserve integration of 
> custom secruity controls with features that exist today:
> 1. support for participation in guice injection
> 2. support for participation in the same configuration system as the rest 
> of the scheduler
> 
> If (1) is not necessary, that simplifies things.  An alternative to (2) 
> is to force custom code to handle its own configuration (e.g. with 
> environment variables).

Perhaps a script example will help evaluating this approach? Bill, would you 
mind giving a simple example of how aurora-scheduler.conf looked had we added a 
custom module in our e2e tests?


- Maxim


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


On Jan. 20, 2016, 8:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42565/
> ---
> 
> (Updated Jan. 20, 2016, 8:21 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch proposes that users installing custom modules do so via a custom 
> main.
> 
> Specifying custom modules on the command line has proven troublesome for 
> replacing the command line args system with one where all arguments are 
> injected and explicitly-defined.  It also adds complexity to the scheduler 
> application by unnecessarily punching holes at specific places.
> 
> If this proposal is agreeable, i will add a NEWS entry and document how one 
> might implement a custom main to add modules.  The tl;dr, however, is to 
> invoke `SchedulerMain.publicMain(customModule)`
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/MoreModules.java 
> d5f96543d1068bf30b9d173a247c2806faf35578 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0659c358479283756179c2cabebc8416730cc3e3 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
>  e32862034a1ad47dae8fff89cb04deb34ccd90e2 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java 
> ccd9a20e8b18831458cba2d53e6b8b84fef06162 
>   src/test/java/org/apache/aurora/scheduler/app/MoreModulesTest.java 
> b2fb3c9dcba64f69a05894f506ba43766f74ddaa 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecuri

Re: Review Request 42656: Don't destroy session between requests with TRequestsTransport

[Aurora ReviewBot]

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

Ship it!


Master (66a4d5f) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Jan. 22, 2016, 4:58 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42656/
> ---
> 
> (Updated Jan. 22, 2016, 4:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> As an API consumer, I want the API client to reuse open connections and not 
> create a new one for every query.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/transport.py 
> 909021ac203185862267d4359d332fc169a06b7e 
>   src/test/python/apache/aurora/common/test_transport.py 
> 1f589a9ae08e1f13be34ad6002ceb11a43fdeb5f 
> 
> Diff: https://reviews.apache.org/r/42656/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest src/test/python::
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Review Request 42656: Don't destroy session between requests with TRequestsTransport

[Kunal Thakar]

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

Review request for Aurora.


Repository: aurora


Description
---

As an API consumer, I want the API client to reuse open connections and not 
create a new one for every query.


Diffs
-

  src/main/python/apache/aurora/common/transport.py 
909021ac203185862267d4359d332fc169a06b7e 
  src/test/python/apache/aurora/common/test_transport.py 
1f589a9ae08e1f13be34ad6002ceb11a43fdeb5f 

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


Testing
---

./pants test.pytest src/test/python::


Thanks,

Kunal Thakar

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Aurora ReviewBot]

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

Ship it!


Master (66a4d5f) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Jan. 22, 2016, 8:11 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42646/
> ---
> 
> (Updated Jan. 22, 2016, 8:11 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Related to a comment i made in https://reviews.apache.org/r/42628/, 
> StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
> you can see from the patch, there's a lot that crumbles with it.
> 
> Since it doesn't show in the patch, StorageBackfill was only filling the 
> `TaskConfig.job` field in tasks and cron jobs:
> https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72
> 
> This backfill has been in place since `0.6.0-incubating` and should be safe 
> to remove.
> https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
> 0d3874e9632952c54ef5ae9aba78638e47c87056 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 376f48545eb860aad5afa028d3b96d04acc08377 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  de4ada431634fb171fab109f1923da810b361205 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 578bb37de8853c4228e76b31f601430b7170946a 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> 609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
>   src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
> 4e4f8d2c0f237c4480abe101835176f7d69958db 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
> 042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 728353116c8892c015a6443d8db09ba241b32230 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> 8fd024a460cd948dab703b99788b1ed2d6c43bc3 
>   src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
> bab45670d66dfed23dd6e0339687166c9be44ba4 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 9fb8aad5d1c0412efc6d1176e543321ebe503e03 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java
>  0768ec37bbc6c3c101aa04a953a36a4af7b25963 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 1ac41d18fd9d603c4c342f9635e116bfd055f858 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 2570464c85630a55d3e6c8653fc0307d54504e15 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 7382eca281eeab17d407ed140f16d6a633d8ad72 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
>  3c4e2bd5eefc141a5d2c5d0e56881899816652f8 
> 
> Diff: https://reviews.apache.org/r/42646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Bill Farner]

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

(Updated Jan. 22, 2016, 12:11 a.m.)


Review request for Aurora, John Sirois and Maxim Khutornenko.


Changes
---

fixup pmd/findbugs errors


Repository: aurora


Description
---

Related to a comment i made in https://reviews.apache.org/r/42628/, 
StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
you can see from the patch, there's a lot that crumbles with it.

Since it doesn't show in the patch, StorageBackfill was only filling the 
`TaskConfig.job` field in tasks and cron jobs:
https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72

This backfill has been in place since `0.6.0-incubating` and should be safe to 
remove.
https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
0d3874e9632952c54ef5ae9aba78638e47c87056 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
376f48545eb860aad5afa028d3b96d04acc08377 
  
src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
 de4ada431634fb171fab109f1923da810b361205 
  src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
578bb37de8853c4228e76b31f601430b7170946a 
  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
  src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
4e4f8d2c0f237c4480abe101835176f7d69958db 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
728353116c8892c015a6443d8db09ba241b32230 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
8fd024a460cd948dab703b99788b1ed2d6c43bc3 
  src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
bab45670d66dfed23dd6e0339687166c9be44ba4 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
9fb8aad5d1c0412efc6d1176e543321ebe503e03 
  
src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 
0768ec37bbc6c3c101aa04a953a36a4af7b25963 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
1ac41d18fd9d603c4c342f9635e116bfd055f858 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
2570464c85630a55d3e6c8653fc0307d54504e15 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
7382eca281eeab17d407ed140f16d6a633d8ad72 
  
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
 3c4e2bd5eefc141a5d2c5d0e56881899816652f8 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 42646: Remove storage backfill and TaskStore mutateTasks.

[Bill Farner]

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

(Updated Jan. 22, 2016, 12:01 a.m.)


Review request for Aurora, John Sirois and Maxim Khutornenko.


Repository: aurora


Description
---

Related to a comment i made in https://reviews.apache.org/r/42628/, 
StorageBackfill was the only call site for `Storage.Mutable.mutateTasks`.  As 
you can see from the patch, there's a lot that crumbles with it.

Since it doesn't show in the patch, StorageBackfill was only filling the 
`TaskConfig.job` field in tasks and cron jobs:
https://github.com/apache/aurora/blob/9ed81a7db58f6a7cb308c8ac6a545705351c8c0e/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java#L72

This backfill has been in place since `0.6.0-incubating` and should be safe to 
remove.
https://github.com/apache/aurora/commit/18ae0ab9696e3565cf57f6a2550c61142e76bee5#diff-74fad6db735428c9b72017b9097bb0c1L124


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
0d3874e9632952c54ef5ae9aba78638e47c87056 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
376f48545eb860aad5afa028d3b96d04acc08377 
  
src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
 de4ada431634fb171fab109f1923da810b361205 
  src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
578bb37de8853c4228e76b31f601430b7170946a 
  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
609a6242e8918bf4a044e6e5e2c8dfe4b900192e 
  src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 
4e4f8d2c0f237c4480abe101835176f7d69958db 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
43fda1d001f52f916b8a6d75fcdb74d4280eaa41 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
042f71dd8bc81d03f2759ee7f7f4b65a098d11e2 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
728353116c8892c015a6443d8db09ba241b32230 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
8fd024a460cd948dab703b99788b1ed2d6c43bc3 
  src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java 
bab45670d66dfed23dd6e0339687166c9be44ba4 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
9fb8aad5d1c0412efc6d1176e543321ebe503e03 
  
src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 
0768ec37bbc6c3c101aa04a953a36a4af7b25963 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
1ac41d18fd9d603c4c342f9635e116bfd055f858 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
2570464c85630a55d3e6c8653fc0307d54504e15 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
7382eca281eeab17d407ed140f16d6a633d8ad72 
  
src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
 3c4e2bd5eefc141a5d2c5d0e56881899816652f8 

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


Testing
---


Thanks,

Bill Farner