Re: Review Request 45467: [PROTOTYPE] Add support for DB migrations and rollbacks.

2016-03-29 Thread Aurora ReviewBot 

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



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

:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java:53:15:
 error: Unused import - 
org.apache.aurora.scheduler.http.H2ConsoleModule.H2_PERM.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:20:14:
 error: Name 'V001_CreateUnifiedContainerTables' must match pattern 
'^[A-Z][a-zA-Z0-9]*$'.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:33:52:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:34:26:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:35:91:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:36:36:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:37:40:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:38:38:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:39:14:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:40:51:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:41:26:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:42:91:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:43:36:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:44:35:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:45:36:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java:51:60:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java:162:81:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java:163:53:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java:164:54:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java:165:56:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java:166:48:
 error: '+' should be on a new line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java:167:32:
 error: '+' should be on a new line.
[ant:checkstyle]

Review Request 45467: [PROTOTYPE] Add support for DB migrations and rollbacks.

2016-03-29 Thread Joshua Cohen 

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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


Repository: aurora


Description
---

THIS CODE IS NOT INTENDED TO BE COMMITTED.

This is just a spike to show a proof of concept for how we can effect automatic 
migrations and rollbacks of the H2 schema. The code is very sloppy, please use 
this to further the discussion on the mailing list about migrations. If we 
agree this methodology is acceptable, I'll clean this up and send out an actual 
review.

That said...

The general gist here is:

1. Use MyBatis Migrations which has built in support for specifying an up and a 
down script for db changes and also makes it easy to locate all existing 
migrations on the classpath.
2. When applying a migration, write the downgrade script to the changelog table 
in the database.
3. Before actually applying migrations, check all changes in the changelog 
table. If the corresponding migration does not exist on the classpath, we 
assume this is a rollback and run the downgrade script from the changelog table 
and delete the corresponding changelog row.


Diffs
-

  build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9 
  examples/vagrant/upstart/aurora-scheduler.conf 
120b89a1dc10a259940cb9527eb2517f19d04471 
  
src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java
 e32862034a1ad47dae8fff89cb04deb34ccd90e2 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 
baf460e987d0a6ba2810507695fe118b6b502f87 
  
src/main/java/org/apache/aurora/scheduler/storage/db/migration/V001_CreateUnifiedContainerTables.java
 PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
6fee2510d044515e0704cf20ec0ba77231050ec4 

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


Testing
---

I manually verified in vagrant that this works as expected for upgrades with 
migrations, upgrades without migrations and rollbacks.


Thanks,

Joshua Cohen

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread Aurora ReviewBot 

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



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

   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=1)
 
   assert hct._total_latency == 0
   assert 
hct.metrics.sample()['total_latency_secs'] == 0
 
   # start the health check (during health check it 
is still 0)
   epsilon = 0.001
   self._clock.tick(1.0 + epsilon)
   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=0.5)
   assert hct._total_latency == 0
   assert 
hct.metrics.sample()['total_latency_secs'] == 0
   assert hct.metrics.sample()['checks'] == 0
 
   # finish the health check
   self._clock.tick(0.5 + epsilon)
   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=1)  # 
interval_secs
 > assert hct._total_latency == 0.5
 E AssertionError: assert 0.5009 
== 0.5
 E  +  where 0.5009 = 
._total_latency
 
 
src/test/python/apache/aurora/executor/common/test_health_checker.py:174: 
AssertionError
 -- Captured stderr call --
 [] Time now: 0.0
 [] Time now: 0.0
 [] Time now: 1.0
 [] Time now: 1.001
 [] Time now: 1.001
 [] Time now: 1.501
 [] Time now: 1.502
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 663 passed, 5 skipped, 1 warnings in 
208.34 seconds 
 
FAILURE


03:17:59 04:24   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 29, 2016, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 10:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread Bill Farner 


> On March 29, 2016, 4:20 p.m., John Sirois wrote:
> > How would these queries produce incorrect results as compared to previous 
> > queries?  Perviously, a check for the collection being null would have had 
> > to have been made by the caller (or a check for !IsSet) to skip the calls 
> > you've added precondition checks to.  The IsSet calls are now gone, meaning 
> > the call sites using those have been semantically adjusted.  I think this 
> > only leaves the callers who pass null of which there should be none due to 
> > the semantic change as well.
> 
> Bill Farner wrote:
> If i understand you correctly - the problem is with call sites that 
> assume `Query.slaveScoped(ImmutableList.of()` translates into `SELECT xyz 
> WHERE slave_host IN ()` (always returning 0 rows).
> 
> However, the semantic is now such that the above query is equivalent to 
> `SELECT xyz WHERE 1`.
> 
> Bill Farner wrote:
> Oh, and to complete the thought - i am assuming that callers never mean 
> `WHERE 1` when they call `Query.slaveScoped(x)`.
> 
> John Sirois wrote:
> I'm assuming the opposite. A generic query builder using this tool might 
> work through criteria, some of which are unset.  The unset criteria should 
> map to an always true ckause (WHERE 1), they should never map to an always 
> false clause (IN ()).  I say this from 10k feet above this code though, not 
> taking real call sites in-mind.  It may be there is no such code and so your 
> checks are fine in practice.
> 
> Bill Farner wrote:
> | It may be there is no such code and so your checks are fine in practice.
> 
> The reverse may also be true :-)
> 
> I honestly don't have a strong opinion on this.  I'm okay if we just want 
> to recognize that the semantic has changed and we need to revisit call sites.

For some anecdotal evidence - i surveyed the call sites (bad on me, it was much 
less effort than i anticipated), and did not find any other 
potentially-vulnerable callers.  So this patch may be much ado about nothing.

Zameer - what do you think?


- Bill


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


On March 29, 2016, 4:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45457/
> ---
> 
> (Updated March 29, 2016, 4:01 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Short of surveying call sites, i don't know the impact of this change.  
> However, my sense is that it's still better to fail fast than produce 
> incorrect results (due to the recent change in semantics of filtering by an 
> empty iterable).
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 
> 
> Diff: https://reviews.apache.org/r/45457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread Bill Farner 

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



@ReviewBot retry

- Bill Farner


On March 29, 2016, 3:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 3:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread Bill Farner 


> On March 29, 2016, 7:56 p.m., Aurora ReviewBot wrote:
> > Master (ec29ac1) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> >    proxy_driver = ProxyDriver()
> >    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 =  > 0x7fd46cb95938>(['./pants', '--pants-distdir=/tmp/user/2/tmpOXtyNI', 
> > 'binary', 'src/main/python/apache/thermos/runner:thermos_runner'])
> >  E+where  > 0x7fd46cb95938> = 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.75/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, 640 passed, 5 skipped, 1 
> > warnings, 8 error in 156.66 seconds 
> >  
> > FAILURE
> > 
> > 
> > 02:56:57 03:42   [complete]
> >FAILURE
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

Not sure what this indicates...trying a wipe of the jenkins workspace and 
rinse/repeat.


- Bill


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


On March 29, 2016, 3:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 3:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread Bill Farner 


> On March 29, 2016, 4:20 p.m., John Sirois wrote:
> > How would these queries produce incorrect results as compared to previous 
> > queries?  Perviously, a check for the collection being null would have had 
> > to have been made by the caller (or a check for !IsSet) to skip the calls 
> > you've added precondition checks to.  The IsSet calls are now gone, meaning 
> > the call sites using those have been semantically adjusted.  I think this 
> > only leaves the callers who pass null of which there should be none due to 
> > the semantic change as well.
> 
> Bill Farner wrote:
> If i understand you correctly - the problem is with call sites that 
> assume `Query.slaveScoped(ImmutableList.of()` translates into `SELECT xyz 
> WHERE slave_host IN ()` (always returning 0 rows).
> 
> However, the semantic is now such that the above query is equivalent to 
> `SELECT xyz WHERE 1`.
> 
> Bill Farner wrote:
> Oh, and to complete the thought - i am assuming that callers never mean 
> `WHERE 1` when they call `Query.slaveScoped(x)`.
> 
> John Sirois wrote:
> I'm assuming the opposite. A generic query builder using this tool might 
> work through criteria, some of which are unset.  The unset criteria should 
> map to an always true ckause (WHERE 1), they should never map to an always 
> false clause (IN ()).  I say this from 10k feet above this code though, not 
> taking real call sites in-mind.  It may be there is no such code and so your 
> checks are fine in practice.

| It may be there is no such code and so your checks are fine in practice.

The reverse may also be true :-)

I honestly don't have a strong opinion on this.  I'm okay if we just want to 
recognize that the semantic has changed and we need to revisit call sites.


- Bill


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


On March 29, 2016, 4:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45457/
> ---
> 
> (Updated March 29, 2016, 4:01 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Short of surveying call sites, i don't know the impact of this change.  
> However, my sense is that it's still better to fail fast than produce 
> incorrect results (due to the recent change in semantics of filtering by an 
> empty iterable).
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 
> 
> Diff: https://reviews.apache.org/r/45457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread Aurora ReviewBot 

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



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

   proxy_driver = ProxyDriver()
   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/2/tmpOXtyNI', 
'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.75/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, 640 passed, 5 skipped, 1 warnings, 8 
error in 156.66 seconds 
 
FAILURE


02:56:57 03:42   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 29, 2016, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 10:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread John Sirois 


> On March 29, 2016, 5:20 p.m., John Sirois wrote:
> > How would these queries produce incorrect results as compared to previous 
> > queries?  Perviously, a check for the collection being null would have had 
> > to have been made by the caller (or a check for !IsSet) to skip the calls 
> > you've added precondition checks to.  The IsSet calls are now gone, meaning 
> > the call sites using those have been semantically adjusted.  I think this 
> > only leaves the callers who pass null of which there should be none due to 
> > the semantic change as well.
> 
> Bill Farner wrote:
> If i understand you correctly - the problem is with call sites that 
> assume `Query.slaveScoped(ImmutableList.of()` translates into `SELECT xyz 
> WHERE slave_host IN ()` (always returning 0 rows).
> 
> However, the semantic is now such that the above query is equivalent to 
> `SELECT xyz WHERE 1`.
> 
> Bill Farner wrote:
> Oh, and to complete the thought - i am assuming that callers never mean 
> `WHERE 1` when they call `Query.slaveScoped(x)`.

I'm assuming the opposite. A generic query builder using this tool might work 
through criteria, some of which are unset.  The unset criteria should map to an 
always true ckause (WHERE 1), they should never map to an always false clause 
(IN ()).  I say this from 10k feet above this code though, not taking real call 
sites in-mind.  It may be there is no such code and so your checks are fine in 
practice.


- John


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


On March 29, 2016, 5:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45457/
> ---
> 
> (Updated March 29, 2016, 5:01 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Short of surveying call sites, i don't know the impact of this change.  
> However, my sense is that it's still better to fail fast than produce 
> incorrect results (due to the recent change in semantics of filtering by an 
> empty iterable).
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 
> 
> Diff: https://reviews.apache.org/r/45457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread Bill Farner 


> On March 29, 2016, 4:20 p.m., John Sirois wrote:
> > How would these queries produce incorrect results as compared to previous 
> > queries?  Perviously, a check for the collection being null would have had 
> > to have been made by the caller (or a check for !IsSet) to skip the calls 
> > you've added precondition checks to.  The IsSet calls are now gone, meaning 
> > the call sites using those have been semantically adjusted.  I think this 
> > only leaves the callers who pass null of which there should be none due to 
> > the semantic change as well.
> 
> Bill Farner wrote:
> If i understand you correctly - the problem is with call sites that 
> assume `Query.slaveScoped(ImmutableList.of()` translates into `SELECT xyz 
> WHERE slave_host IN ()` (always returning 0 rows).
> 
> However, the semantic is now such that the above query is equivalent to 
> `SELECT xyz WHERE 1`.

Oh, and to complete the thought - i am assuming that callers never mean `WHERE 
1` when they call `Query.slaveScoped(x)`.


- Bill


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


On March 29, 2016, 4:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45457/
> ---
> 
> (Updated March 29, 2016, 4:01 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Short of surveying call sites, i don't know the impact of this change.  
> However, my sense is that it's still better to fail fast than produce 
> incorrect results (due to the recent change in semantics of filtering by an 
> empty iterable).
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 
> 
> Diff: https://reviews.apache.org/r/45457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread Bill Farner 

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



@ReviewBot retry

- Bill Farner


On March 29, 2016, 3:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 3:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread Bill Farner 


> On March 29, 2016, 4:20 p.m., John Sirois wrote:
> > How would these queries produce incorrect results as compared to previous 
> > queries?  Perviously, a check for the collection being null would have had 
> > to have been made by the caller (or a check for !IsSet) to skip the calls 
> > you've added precondition checks to.  The IsSet calls are now gone, meaning 
> > the call sites using those have been semantically adjusted.  I think this 
> > only leaves the callers who pass null of which there should be none due to 
> > the semantic change as well.

If i understand you correctly - the problem is with call sites that assume 
`Query.slaveScoped(ImmutableList.of()` translates into `SELECT xyz WHERE 
slave_host IN ()` (always returning 0 rows).

However, the semantic is now such that the above query is equivalent to `SELECT 
xyz WHERE 1`.


- Bill


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


On March 29, 2016, 4:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45457/
> ---
> 
> (Updated March 29, 2016, 4:01 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Short of surveying call sites, i don't know the impact of this change.  
> However, my sense is that it's still better to fail fast than produce 
> incorrect results (due to the recent change in semantics of filtering by an 
> empty iterable).
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 
> 
> Diff: https://reviews.apache.org/r/45457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread Zameer Manji 

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


Ship it!




Ship It!

- Zameer Manji


On March 29, 2016, 3:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 3:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Bill Farner 

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



Went through docs with a fine-toothed comb.  Only ship blocker for me right now 
is that the latest draft doesn't exercise ZK auth in the vagrant example config.


RELEASE-NOTES.md (line 14)


s/Support/Added support/
s/ZK/ZooKeeper/



docs/operations/security.md (line 26)


s/Zookeeper/ZooKeeper/ (capital K)



docs/operations/security.md (line 34)


"To enable authentication for the announcer, see..."



docs/operations/security.md (line 292)


"The Thermos executor can be configured to authenticate with ZooKeeper and 
include an [ACL] on the nodes it creates, which will specify..."



docs/operations/security.md (line 299)


s/ACL/an ACL/



examples/vagrant/announcer-auth.json (line 1)


Whoops, in the last round i intended for you to only _add_ world read 
access.  We should not remove the auth and restricted write/create access - 
these are valuable to exercise in the vagrant environment.



src/main/python/apache/aurora/executor/bin/thermos_executor_main.py (line 201)


feel free to inline this below to avoid the extra var declaration



src/main/python/apache/aurora/executor/common/announcer.py (lines 65 - 70)


Nice!



src/main/python/apache/aurora/executor/common/announcer.py (lines 154 - 164)


Use a list comprehension instead:

```
def to_acl(access):
  return make_acl(...)

default_acl = [to_acl(a) for a in self._zk_auth.acl()]
```



src/main/python/apache/aurora/executor/common/announcer.py (lines 168 - 169)


I assume you do the `[]` -> `None` conversion because the behavior is 
different for these args?

If so, you can make the code more concise and avoid these variable 
reassignments:
```
auth_data = auth_data if auth_data else None
...
default_acl = default_acl if default_acl else None
```

By instead doing this:
```python
return KazooClient(self._ensemble,
   connection_retry=self.DEFAULT_RETRY_POLICY,
   default_acl=default_acl or None,
   auth_data=auth_data or None)
```



src/test/sh/org/apache/aurora/e2e/validate_serverset.py (line 48)


Is this needed?  If so, please include a comment explaining why the timeout 
needs to be at 30.


- Bill Farner


On March 29, 2016, 5:17 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 29, 2016, 5:17 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
>   ]
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
>   examples/vagrant/announcer-auth.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Bill Farner 


> On March 28, 2016, 4:46 p.m., Bill Farner wrote:
> > examples/vagrant/announcer-auth.json, line 11
> > 
> >
> > I now have to backpedal on my advice to store the encrypted credentials 
> > here.  Since our hand is forced to store plaintext for the auth section, we 
> > might as well make this part plaintext too.  That leaves us with the burden 
> > of handling the digest step, but that shouldn't be too bad.
> 
> Kunal Thakar wrote:
> I'd prefer to keep the burden on the configuration provider to keep it 
> simple.

I'm still a -1 to that, but willing to be out-voted by Zameer.

In my opinion, requiring the user to configure the same data (passwords) in 2 
different ways (encrypted and plaintext) introduces unnecessary burden and a 
class of misconfiguration that mere mortals should not be subjected to :-)


- Bill


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


On March 29, 2016, 5:17 p.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 29, 2016, 5:17 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
>   ]
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
>   examples/vagrant/announcer-auth.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
>   src/test/sh/org/apache/aurora/e2e/validate_serverset.py 
> fca1137bd2e7b1306a03dc2a54d2ef15b59af6a8 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Aurora ReviewBot 

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



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

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

- Aurora ReviewBot


On March 30, 2016, 12:17 a.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 30, 2016, 12:17 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
>   ]
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
>   examples/vagrant/announcer-auth.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
>   src/test/sh/org/apache/aurora/e2e/validate_serverset.py 
> fca1137bd2e7b1306a03dc2a54d2ef15b59af6a8 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Kunal Thakar 


> On March 28, 2016, 11:46 p.m., Bill Farner wrote:
> > examples/vagrant/announcer-auth.json, line 11
> > 
> >
> > I now have to backpedal on my advice to store the encrypted credentials 
> > here.  Since our hand is forced to store plaintext for the auth section, we 
> > might as well make this part plaintext too.  That leaves us with the burden 
> > of handling the digest step, but that shouldn't be too bad.

I'd prefer to keep the burden on the configuration provider to keep it simple.


- Kunal


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


On March 30, 2016, 12:17 a.m., Kunal Thakar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45042/
> ---
> 
> (Updated March 30, 2016, 12:17 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add ACL support for announcer
> https://issues.apache.org/jira/browse/AURORA-1643
> 
> Adding support for service discovery ZK authentication. ZK authentication 
> secrets should be stored in a file as json (as follows):
> (Updated JSON format for config file)
> ```json
> {
>   "auth": [
> {
>   "scheme": "",
>   "credential": ""
> }
>   ],
>   "acls": [
> {
>   "scheme": "",
>   "credential": "",
>   "permissions": {
> "read": ,
> "write": ,
> "create": ,
> "delete": ,
> "admin": ,
> "all": 
>   }
> }
>   ]
> }
> ```
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
>   examples/vagrant/announcer-auth.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 120b89a1dc10a259940cb9527eb2517f19d04471 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 6634506108c346f8c23b2da7cc8d20d09d07d590 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
>   src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
>   src/test/sh/org/apache/aurora/e2e/validate_serverset.py 
> fca1137bd2e7b1306a03dc2a54d2ef15b59af6a8 
> 
> Diff: https://reviews.apache.org/r/45042/diff/
> 
> 
> Testing
> ---
> 
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
> /vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kunal Thakar
> 
>

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Kunal Thakar 

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

(Updated March 30, 2016, 12:17 a.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Pystachio'ed the config for easier validation/conversion, removed new e2e test 
and added auth to main e2e test


Repository: aurora


Description
---

Add ACL support for announcer
https://issues.apache.org/jira/browse/AURORA-1643

Adding support for service discovery ZK authentication. ZK authentication 
secrets should be stored in a file as json (as follows):
(Updated JSON format for config file)
```json
{
  "auth": [
{
  "scheme": "",
  "credential": ""
}
  ],
  "acls": [
{
  "scheme": "",
  "credential": "",
  "permissions": {
"read": ,
"write": ,
"create": ,
"delete": ,
"admin": ,
"all": 
  }
}
  ]
}
```


Diffs (updated)
-

  RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
  docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 
  examples/vagrant/announcer-auth.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler.conf 
120b89a1dc10a259940cb9527eb2517f19d04471 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
6634506108c346f8c23b2da7cc8d20d09d07d590 
  src/main/python/apache/aurora/executor/common/announcer.py 
79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 
  src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py 
PRE-CREATION 
  
src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py 
e9f7851292aef3a36da5da9b0fc333a7e7750cf3 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
142b58d5e577c9f4b8e2ae8473cffdea94eba21f 
  src/test/sh/org/apache/aurora/e2e/validate_serverset.py 
fca1137bd2e7b1306a03dc2a54d2ef15b59af6a8 

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


Testing
---

/vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh
/vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Kunal Thakar

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread John Sirois 

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



How would these queries produce incorrect results as compared to previous 
queries?  Perviously, a check for the collection being null would have had to 
have been made by the caller (or a check for !IsSet) to skip the calls you've 
added precondition checks to.  The IsSet calls are now gone, meaning the call 
sites using those have been semantically adjusted.  I think this only leaves 
the callers who pass null of which there should be none due to the semantic 
change as well.

- John Sirois


On March 29, 2016, 5:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45457/
> ---
> 
> (Updated March 29, 2016, 5:01 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Short of surveying call sites, i don't know the impact of this change.  
> However, my sense is that it's still better to fail fast than produce 
> incorrect results (due to the recent change in semantics of filtering by an 
> empty iterable).
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 
> 
> Diff: https://reviews.apache.org/r/45457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread Aurora ReviewBot 

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



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

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On March 29, 2016, 11:01 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45457/
> ---
> 
> (Updated March 29, 2016, 11:01 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Short of surveying call sites, i don't know the impact of this change.  
> However, my sense is that it's still better to fail fast than produce 
> incorrect results (due to the recent change in semantics of filtering by an 
> empty iterable).
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Query.java 
> c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 
> 
> Diff: https://reviews.apache.org/r/45457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread John Sirois 

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


Ship it!




Ship It!

- John Sirois


On March 29, 2016, 4:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 4:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 45456: Use correct query to serve /maintenance.

2016-03-29 Thread Aurora ReviewBot 

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



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

   proxy_driver = ProxyDriver()
   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/tmpbEcb4q', 
'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.75/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, 640 passed, 5 skipped, 1 warnings, 8 
error in 162.95 seconds 
 
FAILURE


23:03:05 03:43   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 29, 2016, 10:46 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45456/
> ---
> 
> (Updated March 29, 2016, 10:46 p.m.)
> 
> 
> Review request for Aurora, John Sirois and Zameer Manji.
> 
> 
> Bugs: AURORA-1652
> https://issues.apache.org/jira/browse/AURORA-1652
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes the 2 issues identified in AURORA-1652.
> 
> I will submit a separate patch to mitigate this issue by preventing 
> error-prone calls to conveniences like `Query.slaveScoped()` with an empty 
> `Iterable` (which has the effect of not filtering).
> 
> 
> Diffs
> -
> 
>   config/legacy_untested_classes.txt afe954fa962b1590e9e6d0c44d62a0f923cb5727 
>   src/main/java/org/apache/aurora/scheduler/http/Maintenance.java 
> 72c8c3e52599ac98bd730e6be6b7b15dffe57cdd 
>   src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45456/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 45457: Mitigate error-prone queries by disallowing no-op filters.

2016-03-29 Thread Bill Farner 

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

Review request for Aurora, John Sirois and Zameer Manji.


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


Repository: aurora


Description
---

Short of surveying call sites, i don't know the impact of this change.  
However, my sense is that it's still better to fail fast than produce incorrect 
results (due to the recent change in semantics of filtering by an empty 
iterable).


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/Query.java 
c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac 

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


Testing
---


Thanks,

Bill Farner

Re: Review Request 45444: Various minor documentation fixes

2016-03-29 Thread Bill Farner 

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


Ship it!




Ship It!

- Bill Farner


On March 29, 2016, 1:10 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45444/
> ---
> 
> (Updated March 29, 2016, 1:10 p.m.)
> 
> 
> Review request for Aurora and Steve Niemitz.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Various minor documentation fixes. Most of them dead links.
> 
> 
> Diffs
> -
> 
>   docs/README.md 50eb7b26f3fbea79c174c06fd8fb3c3888c85b6e 
>   docs/development/client.md a5fee378a87346e63ad628acc8f2dfd3b297120c 
>   docs/development/committers-guide.md 
> 70f67a6a6f6e7ff362e2278ba63deba136742577 
>   docs/development/scheduler.md 66d0857845f180dccbe2cb3bbf3e4fadef655073 
>   docs/features/constraints.md ef2f66479607ad57173107acb742d37cb860d2b6 
>   docs/features/containers.md a0fd4ac6bf260b493e2d32b19df9b551a9f22f54 
>   docs/features/cron-jobs.md 8a864e981c23656c21cac5baf53c096c5833cd40 
>   docs/getting-started/tutorial.md dd3ac0acb64a2ee2827e894dff6dd86020e84239 
>   docs/operations/installation.md 739c322e70230f8ef25ed957322a209935307b0e 
>   docs/reference/configuration-tutorial.md 
> 4390cd6527cb3deabd3a5a79cfad4e0e4737f669 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   docs/reference/scheduler-configuration.md 
> 0b1e3c779b534d2c6e2dbbd720a333fbea1c165f 
> 
> Diff: https://reviews.apache.org/r/45444/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Review Request 45444: Various minor documentation fixes

2016-03-29 Thread Stephan Erb 

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

Review request for Aurora and Steve Niemitz.


Repository: aurora


Description
---

Various minor documentation fixes. Most of them dead links.


Diffs
-

  docs/README.md 50eb7b26f3fbea79c174c06fd8fb3c3888c85b6e 
  docs/development/client.md a5fee378a87346e63ad628acc8f2dfd3b297120c 
  docs/development/committers-guide.md 70f67a6a6f6e7ff362e2278ba63deba136742577 
  docs/development/scheduler.md 66d0857845f180dccbe2cb3bbf3e4fadef655073 
  docs/features/constraints.md ef2f66479607ad57173107acb742d37cb860d2b6 
  docs/features/containers.md a0fd4ac6bf260b493e2d32b19df9b551a9f22f54 
  docs/features/cron-jobs.md 8a864e981c23656c21cac5baf53c096c5833cd40 
  docs/getting-started/tutorial.md dd3ac0acb64a2ee2827e894dff6dd86020e84239 
  docs/operations/installation.md 739c322e70230f8ef25ed957322a209935307b0e 
  docs/reference/configuration-tutorial.md 
4390cd6527cb3deabd3a5a79cfad4e0e4737f669 
  docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
  docs/reference/scheduler-configuration.md 
0b1e3c779b534d2c6e2dbbd720a333fbea1c165f 

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


Testing
---


Thanks,

Stephan Erb

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-29 Thread Aurora ReviewBot 

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


Ship it!




Master (ec29ac1) 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 March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-29 Thread Amol Deshmukh 

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

(Updated March 29, 2016, 10:51 a.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

- \-wfarner
- Doc changes to account for this change.
- RELEASE notes entry to clarify the backward incompatibility wrt bad tier name 
in job configuration.


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


Repository: aurora


Description
---

Introduce "preemptible" flag in TierInfo with backward compatible support for 
"production" flag in TaskConfig.

# Summary of changes
- TierInfo extended to manage a new property: "preemptible"
- If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls back 
to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
- Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
references to ``TaskTestUtil.DEV_TIER``.
- Eager validation of tier specified in TaskConfig in 
``ConfigurationManager.validateAndPopulate(..)``

# Note on backward incompatible change
TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
were silently assigned a default tier. With this change, attempting to schedule 
tasks that specify non-existent tier names will result in an error (see 
``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).


Diffs (updated)
-

  RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
  docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
  src/jmh/java/org/apache/aurora/benchmark/Offers.java 
055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
  src/main/java/org/apache/aurora/scheduler/TierInfo.java 
b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
  src/main/java/org/apache/aurora/scheduler/TierManager.java 
480c4853a6c87dd63a9810ae013e5cfacb29336d 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
97d87ff1b789625f2c07baf7a74a076f07742596 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 9d2bc825edef7fabeccd2334db48acc1bf622eb0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
7f84e90774193b0d31adb7dafcab0a249167cdba 
  src/main/resources/org/apache/aurora/scheduler/tiers.json 
18701058076bedc5d1b667e2b97ad09ce84a9bb9 
  src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
39096af816864ada32a9c07958975740e805f6b0 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
  src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
4fe8c518c580418078275b8056a5c195a765681e 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
a662100ba726cff0e47b4f9650753db9cecdef51 
  src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
e0601c83486671596e412f022ffae78b01c81c9d 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
5103bd0f43d53079976b0f1596e299f2d91aa860 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b6f5e4632ac1e028fdf93da1735463373e2d2788 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
b00add0b2fd4277e196505fffba4440e2e94207e 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
b1c71a6f1847d205c378d0b5a7269ea9d1165be5 

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


Testing
---

# End to end tests
```
$ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
...
*** OK (All tests passed) ***
```

# Jenkins build.sh
```
$ ./build-support/jenkins/build.sh
...
   SUCCESS
```

# Local Scheduler
```
$ ./gradlew run
...
I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
```

# Attempting to schedule job with invalid tier-name
```
vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
job.aurora
 INFO] Creating job test
Job creation failed due to error:
Invalid tier 'badtier' in TaskConfig.
```


Thanks,

Amol Deshmukh

Re: Review Request 45436: Fixes to RELEASE-NOTES for new documentation structure.

2016-03-29 Thread Bill Farner 

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


Ship it!




Ship It!

- Bill Farner


On March 29, 2016, 9:46 a.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45436/
> ---
> 
> (Updated March 29, 2016, 9:46 a.m.)
> 
> 
> Review request for Aurora, Stephan Erb and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes to RELEASE-NOTES for new documentation structure.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 450d363b566c150c6e597b3d188a44c7a77ea891 
> 
> Diff: https://reviews.apache.org/r/45436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> George Sirois
> 
>

Re: Review Request 45436: Fixes to RELEASE-NOTES for new documentation structure.

2016-03-29 Thread Stephan Erb 

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


Ship it!




Ship It!

- Stephan Erb


On March 29, 2016, 6:46 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45436/
> ---
> 
> (Updated March 29, 2016, 6:46 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes to RELEASE-NOTES for new documentation structure.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 450d363b566c150c6e597b3d188a44c7a77ea891 
> 
> Diff: https://reviews.apache.org/r/45436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> George Sirois
> 
>

Re: Review Request 45436: Fixes to RELEASE-NOTES for new documentation structure.

2016-03-29 Thread Aurora ReviewBot 

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


Ship it!




Master (bb16cad) 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 March 29, 2016, 4:46 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45436/
> ---
> 
> (Updated March 29, 2016, 4:46 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes to RELEASE-NOTES for new documentation structure.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 450d363b566c150c6e597b3d188a44c7a77ea891 
> 
> Diff: https://reviews.apache.org/r/45436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> George Sirois
> 
>

Review Request 45436: Fixes to RELEASE-NOTES for new documentation structure.

2016-03-29 Thread George Sirois 

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

Review request for Aurora, Stephan Erb and Bill Farner.


Repository: aurora


Description
---

Fixes to RELEASE-NOTES for new documentation structure.


Diffs
-

  RELEASE-NOTES.md 450d363b566c150c6e597b3d188a44c7a77ea891 

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


Testing
---


Thanks,

George Sirois

Re: Review Request 45396: Adds the ability to set HOME to the sandbox before running the executor.

2016-03-29 Thread Aurora ReviewBot 

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



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

   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=1)
 
   assert hct._total_latency == 0
   assert 
hct.metrics.sample()['total_latency_secs'] == 0
 
   # start the health check (during health check it 
is still 0)
   epsilon = 0.001
   self._clock.tick(1.0 + epsilon)
   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=0.5)
   assert hct._total_latency == 0
   assert 
hct.metrics.sample()['total_latency_secs'] == 0
   assert hct.metrics.sample()['checks'] == 0
 
   # finish the health check
   self._clock.tick(0.5 + epsilon)
   
self._clock.converge(threads=[hct.threaded_health_checker])
   
self._clock.assert_waiting(hct.threaded_health_checker, amount=1)  # 
interval_secs
 > assert hct._total_latency == 0.5
 E AssertionError: assert 0.5009 
== 0.5
 E  +  where 0.5009 = 
._total_latency
 
 
src/test/python/apache/aurora/executor/common/test_health_checker.py:174: 
AssertionError
 -- Captured stderr call --
 [] Time now: 0.0
 [] Time now: 0.0
 [] Time now: 1.0
 [] Time now: 1.001
 [] Time now: 1.001
 [] Time now: 1.501
 [] Time now: 1.502
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 663 passed, 5 skipped, 1 warnings in 
201.06 seconds 
 
FAILURE


16:37:05 04:20   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 29, 2016, 4:18 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> ---
> 
> (Updated March 29, 2016, 4:18 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db 
>   docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45396/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> George Sirois
> 
>

Re: Review Request 45396: Adds the ability to set HOME to the sandbox before running the executor.

2016-03-29 Thread George Sirois 

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

(Updated March 29, 2016, 4:18 p.m.)


Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.


Changes
---

Fixes for style nits.


Repository: aurora


Description
---

In instances where the root filesystem is read-only, it is desirable to
have the executor/runner extract themselves into the sandbox.


Diffs (updated)
-

  RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
  config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db 
  docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142 
  
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 949c299bdbc54f976db994266fb97f3099256f13 
  
src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java
 PRE-CREATION 

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


Testing
---


Thanks,

George Sirois

Re: Review Request 45396: Adds the ability to set HOME to the sandbox before running the executor.

2016-03-29 Thread Joshua Cohen 

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


Ship it!




LGTM, thanks for adding the tests!

If you don't mind fixing the two style nits below I can commit this.


src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 112)


nit: +blank line before



src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java
 (line 71)


same style nit here, blank line after method signature that's broken up 
over multiple lines.


- Joshua Cohen


On March 29, 2016, 5:49 a.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> ---
> 
> (Updated March 29, 2016, 5:49 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db 
>   docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45396/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> George Sirois
> 
>

Re: Review Request 45396: Adds the ability to set HOME to the sandbox before running the executor.

2016-03-29 Thread Aurora ReviewBot 

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


Ship it!




Master (f28f41a) 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 March 29, 2016, 5:49 a.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45396/
> ---
> 
> (Updated March 29, 2016, 5:49 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Steve Niemitz, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> In instances where the root filesystem is read-only, it is desirable to
> have the executor/runner extract themselves into the sandbox.
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 
>   config/legacy_untested_classes.txt 144b258b7a7f73131f07826a0fdcac04834d87db 
>   docs/operations/configuration.md f9e8844914b7af2b0057ca5f1d7d4391a63ca142 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
>  949c299bdbc54f976db994266fb97f3099256f13 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModuleTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45396/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> George Sirois
> 
>