Review Request 46133: Simplify `Credentials`; kill `ZooKeeperClient` dep.

2016-04-12 Thread John Sirois 

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

Review request for Aurora, Bill Farner and Zameer Manji.


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


Repository: aurora


Description
---

The Curator discovery code will need to be configured from the same
command line flags and code as the commons discovery code.  This
simplifies Credentials to be a simple struct and adapts from the
`Credentials.NONE` null-object to use of `Optional` in consumers.

 commons/src/main/java/org/apache/aurora/common/zookeeper/Credentials.java  
   |  90 +++
 commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java  
   | 147 ++
 
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java
 |  15 +++---
 commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java
   |   3 +-
 
commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java
 |  37 +
 
src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 
  |  13 ++---
 src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java  
   |  30 +++
 
src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/ZooKeeperClientModule.java
   |  21 +---
 
src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/flagged/FlaggedClientConfig.java
 |  20 +++
 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
   |   6 +--
 10 files changed, 172 insertions(+), 210 deletions(-)


Diffs
-

  commons/src/main/java/org/apache/aurora/common/zookeeper/Credentials.java 
PRE-CREATION 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java 
41ae035bdd780027f459ec42c39fb6190a963182 
  
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java
 b9eaedb1f6740fdaf65bd0fa2613d4dd914536f7 
  commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 
9127b6e569618f30b74bae2fc7665a92bc30d735 
  
commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java
 537d41e2f6a2d56318e50219c8d848a0e253ecd5 
  
src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 
c14162fa59bec41e00d8604e7659ca3b925f7a23 
  src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
5daafa9234d20dfcfd9a6cc81508836efe39f1f0 
  
src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/ZooKeeperClientModule.java
 4239a867119699267ff8e663a2827feb751fbb79 
  
src/main/java/org/apache/aurora/scheduler/zookeeper/guice/client/flagged/FlaggedClientConfig.java
 5b59d55b8e58d0231a18da9813045a9d1dd70be3 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
89b2813773c2c4bf211883e9d67592b37852776c 

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


Testing
---

Locally green:
```
./gradlew -Pq build
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
```


Thanks,

John Sirois

Re: Review Request 46111: Introduce a Curator-based `SingletonService`.

2016-04-12 Thread Zameer Manji 

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


Ship it!




Ship It!

- Zameer Manji


On April 12, 2016, 2:21 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46111/
> ---
> 
> (Updated April 12, 2016, 2:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
> |   4 ++
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  |   2 +-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>   | 191 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  | 114 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>| 108 +---
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>   | 194 ++
>  6 files changed, 523 insertions(+), 90 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
>  3561d07f65d11060e4a96c9df06af44107a73430 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  0ab24faadcba60ff8e68b2036562e4d39a4ac874 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  559838984a519bdbbb9ed60a71cea726dcd5cb52 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46111/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./gradlew -Pq build`.
> 
> NB: The SingletonServiceImplTest has more coverage and code surrounding 
> illegal state transitions - double advertisement, double leave, advertise 
> after leave (enforcing single-use).  I can add those sorts of checks and test 
> for the checks, but thought I'd leave this 1st RB here to reduce clutter and 
> follow-up if folks want those sorts of checks.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 46111: Introduce a Curator-based `SingletonService`.

2016-04-12 Thread Aurora ReviewBot 

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


Ship it!




Master (3cb599e) 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 April 12, 2016, 9:21 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46111/
> ---
> 
> (Updated April 12, 2016, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
> |   4 ++
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  |   2 +-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>   | 191 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  | 114 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>| 108 +---
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>   | 194 ++
>  6 files changed, 523 insertions(+), 90 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
>  3561d07f65d11060e4a96c9df06af44107a73430 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  0ab24faadcba60ff8e68b2036562e4d39a4ac874 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  559838984a519bdbbb9ed60a71cea726dcd5cb52 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46111/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./gradlew -Pq build`.
> 
> NB: The SingletonServiceImplTest has more coverage and code surrounding 
> illegal state transitions - double advertisement, double leave, advertise 
> after leave (enforcing single-use).  I can add those sorts of checks and test 
> for the checks, but thought I'd leave this 1st RB here to reduce clutter and 
> follow-up if folks want those sorts of checks.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 46111: Introduce a Curator-based `SingletonService`.

2016-04-12 Thread John Sirois 

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



@ReviewBot retry

- John Sirois


On April 12, 2016, 3:21 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46111/
> ---
> 
> (Updated April 12, 2016, 3:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
> |   4 ++
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  |   2 +-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>   | 191 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  | 114 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>| 108 +---
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>   | 194 ++
>  6 files changed, 523 insertions(+), 90 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
>  3561d07f65d11060e4a96c9df06af44107a73430 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  0ab24faadcba60ff8e68b2036562e4d39a4ac874 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  559838984a519bdbbb9ed60a71cea726dcd5cb52 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46111/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./gradlew -Pq build`.
> 
> NB: The SingletonServiceImplTest has more coverage and code surrounding 
> illegal state transitions - double advertisement, double leave, advertise 
> after leave (enforcing single-use).  I can add those sorts of checks and test 
> for the checks, but thought I'd leave this 1st RB here to reduce clutter and 
> follow-up if folks want those sorts of checks.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Re: Review Request 46111: Introduce a Curator-based `SingletonService`.

2016-04-12 Thread Aurora ReviewBot 

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



Master (3cb599e) is red with this patch.
  ./build-support/jenkins/build.sh

   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/user/10021/tmp_3YC6Y', 
'binary', 'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.80/bin/pants", 
line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  16 failed, 631 passed, 6 skipped, 1 warnings, 8 
error in 195.35 seconds 
 
FAILURE


   Waiting for background workers to finish.
22:06:52 04:54   [complete]
   FAILURE


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

- Aurora ReviewBot


On April 12, 2016, 9:21 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46111/
> ---
> 
> (Updated April 12, 2016, 9:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
> |   4 ++
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  |   2 +-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>   | 191 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  | 114 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>| 108 +---
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>   | 194 ++
>  6 files changed, 523 insertions(+), 90 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
>  3561d07f65d11060e4a96c9df06af44107a73430 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  0ab24faadcba60ff8e68b2036562e4d39a4ac874 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  PRE-CREATION 
>   
>

Re: Review Request 46111: Introduce a Curator-based `SingletonService`.

2016-04-12 Thread John Sirois 

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



Helpful reference links:
The leader latch cache recipe used in place of commons Candidate: 
http://curator.apache.org/curator-recipes/leader-latch.html
The persitent node recipe used in place of commons ServerSet.join: 
http://curator.apache.org/curator-recipes/persistent-ephemeral-node.html
The APIs (for 3.1.0, but all the relevant bits used are the same): 
http://curator.apache.org/apidocs/index.html

- John Sirois


On April 12, 2016, 3:21 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46111/
> ---
> 
> (Updated April 12, 2016, 3:21 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
> |   4 ++
>  
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  |   2 +-
>  
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>   | 191 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  | 114 +
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>| 108 +---
>  
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>   | 194 ++
>  6 files changed, 523 insertions(+), 90 deletions(-)
> 
> 
> Diffs
> -
> 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java
>  3561d07f65d11060e4a96c9df06af44107a73430 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
>  0ab24faadcba60ff8e68b2036562e4d39a4ac874 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
>  559838984a519bdbbb9ed60a71cea726dcd5cb52 
>   
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46111/diff/
> 
> 
> Testing
> ---
> 
> Locally green: `./gradlew -Pq build`.
> 
> NB: The SingletonServiceImplTest has more coverage and code surrounding 
> illegal state transitions - double advertisement, double leave, advertise 
> after leave (enforcing single-use).  I can add those sorts of checks and test 
> for the checks, but thought I'd leave this 1st RB here to reduce clutter and 
> follow-up if folks want those sorts of checks.
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Review Request 46111: Introduce a Curator-based `SingletonService`.

2016-04-12 Thread John Sirois 

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

Review request for Aurora, Bill Farner and Zameer Manji.


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


Repository: aurora


Description
---

commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java  
  |   4 ++
 
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
 |   2 +-
 
src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
  | 191 +
 
src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
 | 114 +
 
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
   | 108 +---
 
src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
  | 194 ++
 6 files changed, 523 insertions(+), 90 deletions(-)


Diffs
-

  
commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonService.java 
3561d07f65d11060e4a96c9df06af44107a73430 
  
commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java
 0ab24faadcba60ff8e68b2036562e4d39a4ac874 
  
src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java
 559838984a519bdbbb9ed60a71cea726dcd5cb52 
  
src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java
 PRE-CREATION 

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


Testing
---

Locally green: `./gradlew -Pq build`.

NB: The SingletonServiceImplTest has more coverage and code surrounding illegal 
state transitions - double advertisement, double leave, advertise after leave 
(enforcing single-use).  I can add those sorts of checks and test for the 
checks, but thought I'd leave this 1st RB here to reduce clutter and follow-up 
if folks want those sorts of checks.


Thanks,

John Sirois

Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

2016-04-12 Thread Aurora ReviewBot 

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


Ship it!




Master (a4853a4) 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 April 12, 2016, 6:09 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> ---
> 
> (Updated April 12, 2016, 6:09 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> First step towards consolidating resource definitions: getting rid of custom 
> ResourceVector enum.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

2016-04-12 Thread Maxim Khutornenko 

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

(Updated April 12, 2016, 6:09 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Bill's comments.


Repository: aurora


Description
---

First step towards consolidating resource definitions: getting rid of custom 
ResourceVector enum.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
1d9d5fc98494892b7d90e97f93db267d42ba7619 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

2016-04-12 Thread Zameer Manji 

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


Ship it!




Ship It!

- Zameer Manji


On April 11, 2016, 5:03 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> ---
> 
> (Updated April 11, 2016, 5:03 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> First step towards consolidating resource definitions: getting rid of custom 
> ResourceVector enum.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 46103: Revert "Revert "Add support for storing and fetching images as properties of task configs.""

2016-04-12 Thread Joshua Cohen 

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

(Updated April 12, 2016, 5:37 p.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

- Add explicit instructions to make DB changes to schema.sql *and* to the 
migrations in db-migrations.md
- Add `IF NOT EXISTS` to drop table statements.
- Fix spacing in mapper xml.


Repository: aurora


Description
---

This reverts commit b5c9e1bc46a623b5d898ec4dacbe132b79903dd7 and layers the 
changes needed to support DB migrations on top as well.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
fc038d7950fb3c9379ab52d1fc32433ce9f692d1 
  config/checkstyle/suppressions.xml d9fe92a786b9f077d915a516d21217a7c0cd5027 
  docs/development/db-migration.md bddfee127c46072153a5c4c46ffdff0e507ae206 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 68573ba15ff886ca0a47f09decda3b6743b80243 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
7c1127e537133670dd6e04045c1779a3aee4f2c3 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
364026a8ef2b47cf1beafa3990691d3375516fe6 
  src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
12ca16b79a062d9ea15c206ef963fb077ad7ad98 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateAppcImagesTable.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V002_CreateDockerImagesTable.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
eb848add00fba6d3571657bb9080be0599b2756a 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
52189677a0ad81a26ee4887d1f49cb449c32872c 
  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 ecd48ecc87428961b879aede0005a009d82d0858 
  
src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java 
c316e497a34a45c7ada2ca83a1115e826c0f572f 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
5895afad6952357afed592580d15c05fe526003f 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 
08530397ff75081bde6f07f9d53317b5486e0da4 

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


Testing
---

./gradlew build -Pq
Ran e2e tests.


Thanks,

Joshua Cohen

Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

2016-04-12 Thread Bill Farner 


> On April 11, 2016, 6:39 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java, line 
> > 31
> > 
> >
> > It's probably time for some constants to justify these constants (or at 
> > least admit that they are lies!).
> 
> Maxim Khutornenko wrote:
> Not sure I follow, these _are_ the constants baked into the enum with the 
> intention to hide all definitions inside this class and anonymize values via 
> getters. Exposing global constants here will defeat the purpose of this 
> change.

Ugh, sorry - haste on my part.  *Comments*, not constants :-/


- Bill


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


On April 11, 2016, 5:03 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> ---
> 
> (Updated April 11, 2016, 5:03 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> First step towards consolidating resource definitions: getting rid of custom 
> ResourceVector enum.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 46103: Revert "Revert "Add support for storing and fetching images as properties of task configs.""

2016-04-12 Thread Maxim Khutornenko 

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


Ship it!





docs/development/db-migration.md (lines 30 - 33)


Probably worth mentioning explicitly that schema.sql should always be kept 
up-to-date to ensure clean scheduler deploy always works.



src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateAppcImagesTable.java
 (line 44)


I'd use `DROP TABLE IF EXISTS` instead as a more robust default.



src/main/java/org/apache/aurora/scheduler/storage/db/migration/V002_CreateDockerImagesTable.java
 (line 44)


same here



src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 
(line 336)


spacing seems to be off


- Maxim Khutornenko


On April 12, 2016, 4:22 p.m., Joshua Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46103/
> ---
> 
> (Updated April 12, 2016, 4:22 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This reverts commit b5c9e1bc46a623b5d898ec4dacbe132b79903dd7 and layers the 
> changes needed to support DB migrations on top as well.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> fc038d7950fb3c9379ab52d1fc32433ce9f692d1 
>   config/checkstyle/suppressions.xml d9fe92a786b9f077d915a516d21217a7c0cd5027 
>   docs/development/db-migration.md bddfee127c46072153a5c4c46ffdff0e507ae206 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  68573ba15ff886ca0a47f09decda3b6743b80243 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 
> 7c1127e537133670dd6e04045c1779a3aee4f2c3 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 364026a8ef2b47cf1beafa3990691d3375516fe6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 12ca16b79a062d9ea15c206ef963fb077ad7ad98 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateAppcImagesTable.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V002_CreateDockerImagesTable.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbImage.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> eb848add00fba6d3571657bb9080be0599b2756a 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  52189677a0ad81a26ee4887d1f49cb449c32872c 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> be60c3be0cd51d06ba0ca7f56384281ee71d8c55 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  ecd48ecc87428961b879aede0005a009d82d0858 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  c316e497a34a45c7ada2ca83a1115e826c0f572f 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 5895afad6952357afed592580d15c05fe526003f 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  08530397ff75081bde6f07f9d53317b5486e0da4 
> 
> Diff: https://reviews.apache.org/r/46103/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> Ran e2e tests.
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>

Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

2016-04-12 Thread Maxim Khutornenko 


> On April 12, 2016, 1:39 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java, line 
> > 31
> > 
> >
> > It's probably time for some constants to justify these constants (or at 
> > least admit that they are lies!).

Not sure I follow, these _are_ the constants baked into the enum with the 
intention to hide all definitions inside this class and anonymize values via 
getters. Exposing global constants here will defeat the purpose of this change.


- Maxim


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


On April 12, 2016, 12:03 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> ---
> 
> (Updated April 12, 2016, 12:03 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> First step towards consolidating resource definitions: getting rid of custom 
> ResourceVector enum.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 46098: AURORA-1584: Aurora 0.13.0 release candidate missing CHANGELOG

2016-04-12 Thread Stephan Erb 

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


Ship it!




Ship It!

- Stephan Erb


On April 12, 2016, 5:03 p.m., Jake Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46098/
> ---
> 
> (Updated April 12, 2016, 5:03 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Bill Farner.
> 
> 
> Bugs: AURORA-1584
> https://issues.apache.org/jira/browse/AURORA-1584
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updates CHANGELOG missed when creating the 0.13.0-rc0 release candidate.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 153faaea67c9c259f7e5970248a526f3cee21e1d 
> 
> Diff: https://reviews.apache.org/r/46098/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jake Farrell
> 
>

Re: Review Request 46098: AURORA-1584: Aurora 0.13.0 release candidate missing CHANGELOG

2016-04-12 Thread John Sirois 

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


Ship it!




Ship It!

- John Sirois


On April 12, 2016, 9:03 a.m., Jake Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46098/
> ---
> 
> (Updated April 12, 2016, 9:03 a.m.)
> 
> 
> Review request for Aurora, John Sirois and Bill Farner.
> 
> 
> Bugs: AURORA-1584
> https://issues.apache.org/jira/browse/AURORA-1584
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updates CHANGELOG missed when creating the 0.13.0-rc0 release candidate.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 153faaea67c9c259f7e5970248a526f3cee21e1d 
> 
> Diff: https://reviews.apache.org/r/46098/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jake Farrell
> 
>

Review Request 46098: AURORA-1584: Aurora 0.13.0 release candidate missing CHANGELOG

2016-04-12 Thread Jake Farrell 

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

Review request for Aurora, John Sirois and Bill Farner.


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


Repository: aurora


Description
---

Updates CHANGELOG missed when creating the 0.13.0-rc0 release candidate.


Diffs
-

  CHANGELOG 153faaea67c9c259f7e5970248a526f3cee21e1d 

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


Testing
---


Thanks,

Jake Farrell