Re: Review Request 36305: Remove restriction that -hostname must resolve.

2015-07-08 Thread Aurora ReviewBot

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


Master (62a432a) 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 July 8, 2015, 12:53 p.m., Brian Brazil wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36305/
 ---
 
 (Updated July 8, 2015, 12:53 p.m.)
 
 
 Review request for Aurora, Stephan Erb and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 There's valid configuration where the hostname
 visibile in the browser won't exist in DNS, and
 mesos master/slave's equivilent settings don't
 have this restriction.
 
 Leave as a warning, as for most users this is a misconfiguration.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 b4701295922ddad44d9d5c9fa29eb95e195e1604 
 
 Diff: https://reviews.apache.org/r/36305/diff/
 
 
 Testing
 ---
 
 Verified in Vagrant environment.
 
 
 Thanks,
 
 Brian Brazil
 




Review Request 36305: Remove restriction that -hostname must resolve.

2015-07-08 Thread Brian Brazil

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

Review request for Aurora, Stephan Erb and Bill Farner.


Repository: aurora


Description
---

There's valid configuration where the hostname
visibile in the browser won't exist in DNS, and
mesos master/slave's equivilent settings don't
have this restriction.

Leave as a warning, as for most users this is a misconfiguration.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
b4701295922ddad44d9d5c9fa29eb95e195e1604 

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


Testing
---

Verified in Vagrant environment.


Thanks,

Brian Brazil



Re: Review Request 36320: Enable debugging tools on the scheduler in vagrant.

2015-07-08 Thread Aurora ReviewBot

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

Ship it!


Master (62a432a) 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 July 8, 2015, 5:57 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36320/
 ---
 
 (Updated July 8, 2015, 5:57 p.m.)
 
 
 Review request for Aurora and Zameer Manji.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This enables remote debugging on the JVM, allowing you to attach by default 
 when developing.  This also enables the H2 console, which is extremely useful 
 for debugging DB state.
 
 
 Diffs
 -
 
   examples/vagrant/upstart/aurora-scheduler.conf 
 1c2390f0248e91e65a548e67f6af1be8d2526b0a 
 
 Diff: https://reviews.apache.org/r/36320/diff/
 
 
 Testing
 ---
 
 Ran in vagrant.
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 36064: Perform leader redirect using HTTP status code 307

2015-07-08 Thread Stephan Erb

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


Friendly ping. Anything still missing here?

- Stephan Erb


On July 5, 2015, 9:38 p.m., Stephan Erb wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36064/
 ---
 
 (Updated July 5, 2015, 9:38 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Perform leader redirect using HTTP status code 307.
 
 The scheduler used to redirect to its leading master using the HTTP status 
 code `302 Found`. Many browsers (including utilities like curl and python 
 requests) implement this status code wrongly. They are changing all PUT and 
 POST requests to GET (see https://en.wikipedia.org/wiki/HTTP_302). By using 
 the the new HTTP/1.1 code `307 Temporary Redirect` we remedy this situation.
 
 Aurora is already using status code `307` within the MNAME-Feature. By using 
 it also for the leader redirect, we enable users to run commands like `curl 
 -X PUT http://non-leading-master/mname/...`
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java 
 1c2b08de1e2c470d32baeb845ae0e0a7ce2b75aa 
   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
 f8b134fa1f3fbbd7affef31acb077e7acd4f9761 
 
 Diff: https://reviews.apache.org/r/36064/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Stephan Erb
 




Review Request 36319: DB task store: Remove invalid constraint on task_config_metadata.

2015-07-08 Thread Bill Farner

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

Review request for Aurora and Zameer Manji.


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


Repository: aurora


Description
---

DB task store: Remove invalid constraint on task_config_metadata.


Diffs
-

  src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
687010e6aa49c18c6c2df6a1cf682d54340dd040 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
edfeb099861a0005231b5f6f245fd50906c24c4b 

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


Testing
---


Thanks,

Bill Farner



Re: Review Request 36320: Enable debugging tools on the scheduler in vagrant.

2015-07-08 Thread Bill Farner

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

(Updated July 8, 2015, 5:57 p.m.)


Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

This enables remote debugging on the JVM, allowing you to attach by default 
when developing.  This also enables the H2 console, which is extremely useful 
for debugging DB state.


Diffs (updated)
-

  examples/vagrant/upstart/aurora-scheduler.conf 
1c2390f0248e91e65a548e67f6af1be8d2526b0a 

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


Testing
---

Ran in vagrant.


Thanks,

Bill Farner



Re: Review Request 36319: DB task store: Remove invalid constraint on task_config_metadata.

2015-07-08 Thread Aurora ReviewBot

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

Ship it!


Master (62a432a) 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 July 8, 2015, 5:46 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36319/
 ---
 
 (Updated July 8, 2015, 5:46 p.m.)
 
 
 Review request for Aurora and Zameer Manji.
 
 
 Bugs: AURORA-1383
 https://issues.apache.org/jira/browse/AURORA-1383
 
 
 Repository: aurora
 
 
 Description
 ---
 
 DB task store: Remove invalid constraint on task_config_metadata.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
 687010e6aa49c18c6c2df6a1cf682d54340dd040 
   
 src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
 edfeb099861a0005231b5f6f245fd50906c24c4b 
 
 Diff: https://reviews.apache.org/r/36319/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 36064: Perform leader redirect using HTTP status code 307

2015-07-08 Thread Joshua Cohen

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

Ship it!


Apologies, I was moving across the country last week and this got buried.

Looks fine to me!

- Joshua Cohen


On July 5, 2015, 7:38 p.m., Stephan Erb wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36064/
 ---
 
 (Updated July 5, 2015, 7:38 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Perform leader redirect using HTTP status code 307.
 
 The scheduler used to redirect to its leading master using the HTTP status 
 code `302 Found`. Many browsers (including utilities like curl and python 
 requests) implement this status code wrongly. They are changing all PUT and 
 POST requests to GET (see https://en.wikipedia.org/wiki/HTTP_302). By using 
 the the new HTTP/1.1 code `307 Temporary Redirect` we remedy this situation.
 
 Aurora is already using status code `307` within the MNAME-Feature. By using 
 it also for the leader redirect, we enable users to run commands like `curl 
 -X PUT http://non-leading-master/mname/...`
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java 
 1c2b08de1e2c470d32baeb845ae0e0a7ce2b75aa 
   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
 f8b134fa1f3fbbd7affef31acb077e7acd4f9761 
 
 Diff: https://reviews.apache.org/r/36064/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Stephan Erb
 




Re: Review Request 36319: DB task store: Remove invalid constraint on task_config_metadata.

2015-07-08 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On July 8, 2015, 10:46 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36319/
 ---
 
 (Updated July 8, 2015, 10:46 a.m.)
 
 
 Review request for Aurora and Zameer Manji.
 
 
 Bugs: AURORA-1383
 https://issues.apache.org/jira/browse/AURORA-1383
 
 
 Repository: aurora
 
 
 Description
 ---
 
 DB task store: Remove invalid constraint on task_config_metadata.
 
 
 Diffs
 -
 
   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
 687010e6aa49c18c6c2df6a1cf682d54340dd040 
   
 src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
 edfeb099861a0005231b5f6f245fd50906c24c4b 
 
 Diff: https://reviews.apache.org/r/36319/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Review Request 36320: Enable debugging tools on the scheduler in vagrant.

2015-07-08 Thread Bill Farner

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

Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

This enables remote debugging on the JVM, allowing you to attach by default 
when developing.  This also enables the H2 console, which is extremely useful 
for debugging DB state.


Diffs
-

  examples/vagrant/upstart/aurora-scheduler.conf 
1c2390f0248e91e65a548e67f6af1be8d2526b0a 
  src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
82d479e142afe798cd84135b981075035d9ca6dc 

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


Testing
---

Ran in vagrant.


Thanks,

Bill Farner



Re: Review Request 36320: Enable debugging tools on the scheduler in vagrant.

2015-07-08 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On July 8, 2015, 10:57 a.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36320/
 ---
 
 (Updated July 8, 2015, 10:57 a.m.)
 
 
 Review request for Aurora and Zameer Manji.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This enables remote debugging on the JVM, allowing you to attach by default 
 when developing.  This also enables the H2 console, which is extremely useful 
 for debugging DB state.
 
 
 Diffs
 -
 
   examples/vagrant/upstart/aurora-scheduler.conf 
 1c2390f0248e91e65a548e67f6af1be8d2526b0a 
 
 Diff: https://reviews.apache.org/r/36320/diff/
 
 
 Testing
 ---
 
 Ran in vagrant.
 
 
 Thanks,
 
 Bill Farner
 




Review Request 36329: Add a stat that tracks uncaught exceptions in pubsub event handlers.

2015-07-08 Thread Bill Farner

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

Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

Prior to this patch, it's relatively easy for exceptions thrown by pubsub 
handlers to land quietly in the log, and with generic messages (which don't 
include a stack trace).  This gives us a stat to alert on, and improves our 
ability to hone in on an issue when it arises.

This includes some binding cleanup to make plumbing easier, resulting in less 
code overall.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
82d479e142afe798cd84135b981075035d9ca6dc 
  src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
c6295e58a93966fb126be20bbdc6a18dc2cdca56 

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


Testing
---


Thanks,

Bill Farner



Re: Review Request 36305: Remove restriction that -hostname must resolve.

2015-07-08 Thread Stephan Erb

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

Ship it!


Ship It!

- Stephan Erb


On July 8, 2015, 2:53 p.m., Brian Brazil wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36305/
 ---
 
 (Updated July 8, 2015, 2:53 p.m.)
 
 
 Review request for Aurora, Stephan Erb and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 There's valid configuration where the hostname
 visibile in the browser won't exist in DNS, and
 mesos master/slave's equivilent settings don't
 have this restriction.
 
 Leave as a warning, as for most users this is a misconfiguration.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 b4701295922ddad44d9d5c9fa29eb95e195e1604 
 
 Diff: https://reviews.apache.org/r/36305/diff/
 
 
 Testing
 ---
 
 Verified in Vagrant environment.
 
 
 Thanks,
 
 Brian Brazil
 




Re: Review Request 36329: Add a stat that tracks uncaught exceptions in pubsub event handlers.

2015-07-08 Thread Bill Farner

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

(Updated July 8, 2015, 8:57 p.m.)


Review request for Aurora and Zameer Manji.


Changes
---

Fixes unexpected brittle tests that surfaced.  Sorry for pulling the trigger 
early on the first draft.


Repository: aurora


Description
---

Prior to this patch, it's relatively easy for exceptions thrown by pubsub 
handlers to land quietly in the log, and with generic messages (which don't 
include a stack trace).  This gives us a stat to alert on, and improves our 
ability to hone in on an issue when it arises.

This includes some binding cleanup to make plumbing easier, resulting in less 
code overall.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
82d479e142afe798cd84135b981075035d9ca6dc 
  src/test/java/org/apache/aurora/scheduler/async/KillRetryTest.java 
0faee9279bc39b7e71f3d4cd12f6b21dcc678356 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
9afd7dfd4b283bd35785003983030865e2311731 
  src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
c6295e58a93966fb126be20bbdc6a18dc2cdca56 
  
src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
 cd85b80b742af094b0ba0dd4932da29e7487e846 

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


Testing
---


Thanks,

Bill Farner



Re: Review Request 36329: Add a stat that tracks uncaught exceptions in pubsub event handlers.

2015-07-08 Thread Aurora ReviewBot

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

Ship it!


Master (144cf60) 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 July 8, 2015, 8:57 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36329/
 ---
 
 (Updated July 8, 2015, 8:57 p.m.)
 
 
 Review request for Aurora and Zameer Manji.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Prior to this patch, it's relatively easy for exceptions thrown by pubsub 
 handlers to land quietly in the log, and with generic messages (which don't 
 include a stack trace).  This gives us a stat to alert on, and improves our 
 ability to hone in on an issue when it arises.
 
 This includes some binding cleanup to make plumbing easier, resulting in less 
 code overall.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
 82d479e142afe798cd84135b981075035d9ca6dc 
   src/test/java/org/apache/aurora/scheduler/async/KillRetryTest.java 
 0faee9279bc39b7e71f3d4cd12f6b21dcc678356 
   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
 9afd7dfd4b283bd35785003983030865e2311731 
   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
 c6295e58a93966fb126be20bbdc6a18dc2cdca56 
   
 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
  cd85b80b742af094b0ba0dd4932da29e7487e846 
 
 Diff: https://reviews.apache.org/r/36329/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 36305: Remove restriction that -hostname must resolve.

2015-07-08 Thread Bill Farner

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

Ship it!



src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java (line 154)
https://reviews.apache.org/r/36305/#comment144195

Some more prose would be useful.  How about:

Unable to resolve name specified in -hostname.  Depending on your 
environment, this may be valid.


- Bill Farner


On July 8, 2015, 12:53 p.m., Brian Brazil wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36305/
 ---
 
 (Updated July 8, 2015, 12:53 p.m.)
 
 
 Review request for Aurora, Stephan Erb and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 There's valid configuration where the hostname
 visibile in the browser won't exist in DNS, and
 mesos master/slave's equivilent settings don't
 have this restriction.
 
 Leave as a warning, as for most users this is a misconfiguration.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
 b4701295922ddad44d9d5c9fa29eb95e195e1604 
 
 Diff: https://reviews.apache.org/r/36305/diff/
 
 
 Testing
 ---
 
 Verified in Vagrant environment.
 
 
 Thanks,
 
 Brian Brazil
 




Re: Review Request 34337: Add Docker Parameters

2015-07-08 Thread Bill Farner

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


Only nits remaining, and one request for test coverage.

One final disclaimer on the security issue this creates - IIUC, arbitrary 
user-specified volume mounts opens up your cluster to privilege escalation.
See this discussion for some detail: 
https://github.com/docker/docker/issues/3124, specifically this comment:
```
 thaJeztah commented on May 23

@JWGmeligMeyling files and folders created in the volume will have the same 
uid:gid (numeric) as the user creating them in the container. If you add a user 
inside the container having the same uid:gid as outside the container and run 
your contsiner as that user, that should be possible
```

More direct coverage of the risk:
https://fosterelli.co/privilege-escalation-via-docker.html
http://reventlov.com/advisories/using-the-docker-command-to-root-the-host


I'm happy to be proven wrong on this suspicion, but please confirm for yourself 
that this is safe to do.


src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (line 66)
https://reviews.apache.org/r/34337/#comment144179

Matching the terminology above, how about s/enable/allow/?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 186)
https://reviews.apache.org/r/34337/#comment144183

I believe this throws an NPE when parameters is not set.  Can you 
confirm/deny in a unit test case?



src/main/python/apache/aurora/config/schema/base.py (line 97)
https://reviews.apache.org/r/34337/#comment144186

I believe this should be
```
parameters = Default(List(Parameter), [])
```

to avoid requiring the argument.



src/main/python/apache/aurora/config/thrift.py (lines 133 - 137)
https://reviews.apache.org/r/34337/#comment144188

Please add a test case for this in
`src/test/python/apache/aurora/config/test_thrift.py`


- Bill Farner


On July 5, 2015, 11:58 p.m., Mauricio Garavaglia wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34337/
 ---
 
 (Updated July 5, 2015, 11:58 p.m.)
 
 
 Review request for Aurora and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Support Arbitrary Docker Parameters in DockerContainer
 
 
 Diffs
 -
 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift d740a90 
   docs/configuration-reference.md dafd306 
   
 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
  be79e70 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 c0d165a 
   src/main/python/apache/aurora/config/schema/base.py d1f1e4f 
   src/main/python/apache/aurora/config/thrift.py 88dd1c7 
   
 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
 c0cadfb 
 
 Diff: https://reviews.apache.org/r/34337/diff/
 
 
 Testing
 ---
 
 Used Docker as the container of a Job. Included volumes and label parameters 
 which are correctly picked up by mesos when starting the task. The docker 
 container gets the specified label and bind mounts the volumes correctly. 
 I've been running multiple PostgreSQL databases docker containers for several 
 weeks deploying them as aurora jobs.
 
 
 Thanks,
 
 Mauricio Garavaglia
 




Re: Review Request 36064: Perform leader redirect using HTTP status code 307

2015-07-08 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On July 5, 2015, 7:38 p.m., Stephan Erb wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36064/
 ---
 
 (Updated July 5, 2015, 7:38 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Perform leader redirect using HTTP status code 307.
 
 The scheduler used to redirect to its leading master using the HTTP status 
 code `302 Found`. Many browsers (including utilities like curl and python 
 requests) implement this status code wrongly. They are changing all PUT and 
 POST requests to GET (see https://en.wikipedia.org/wiki/HTTP_302). By using 
 the the new HTTP/1.1 code `307 Temporary Redirect` we remedy this situation.
 
 Aurora is already using status code `307` within the MNAME-Feature. By using 
 it also for the leader redirect, we enable users to run commands like `curl 
 -X PUT http://non-leading-master/mname/...`
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java 
 1c2b08de1e2c470d32baeb845ae0e0a7ce2b75aa 
   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
 f8b134fa1f3fbbd7affef31acb077e7acd4f9761 
 
 Diff: https://reviews.apache.org/r/36064/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Stephan Erb
 




Re: Review Request 36329: Add a stat that tracks uncaught exceptions in pubsub event handlers.

2015-07-08 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On July 8, 2015, 1:57 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36329/
 ---
 
 (Updated July 8, 2015, 1:57 p.m.)
 
 
 Review request for Aurora and Zameer Manji.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Prior to this patch, it's relatively easy for exceptions thrown by pubsub 
 handlers to land quietly in the log, and with generic messages (which don't 
 include a stack trace).  This gives us a stat to alert on, and improves our 
 ability to hone in on an issue when it arises.
 
 This includes some binding cleanup to make plumbing easier, resulting in less 
 code overall.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
 82d479e142afe798cd84135b981075035d9ca6dc 
   src/test/java/org/apache/aurora/scheduler/async/KillRetryTest.java 
 0faee9279bc39b7e71f3d4cd12f6b21dcc678356 
   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
 9afd7dfd4b283bd35785003983030865e2311731 
   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
 c6295e58a93966fb126be20bbdc6a18dc2cdca56 
   
 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
  cd85b80b742af094b0ba0dd4932da29e7487e846 
 
 Diff: https://reviews.apache.org/r/36329/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 36338: Don't reject update requests scoped to already-updated instances.

2015-07-08 Thread Aurora ReviewBot

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

Ship it!


Master (8efcd06) 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 July 8, 2015, 11:33 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36338/
 ---
 
 (Updated July 8, 2015, 11:33 p.m.)
 
 
 Review request for Aurora and Zameer Manji.
 
 
 Bugs: AURORA-1332
 https://issues.apache.org/jira/browse/AURORA-1332
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Don't reject update requests scoped to already-updated instances.
 
 
 Diffs
 -
 
   
 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
  12a1732a9fafde3d4180fde095ca57f35373771b 
   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 
 bd3196318b4fbd9f5fe5deb8294b5007f1ba93f0 
   
 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
  a5ca21bf1e53cce1573d4512d4048d1a37ed50de 
   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
 d0deada3dc5335b2b6c2794256e8429ef9a4646c 
 
 Diff: https://reviews.apache.org/r/36338/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Bill Farner
 




Review Request 36338: Don't reject update requests scoped to already-updated instances.

2015-07-08 Thread Bill Farner

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

Review request for Aurora and Zameer Manji.


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


Repository: aurora


Description
---

Don't reject update requests scoped to already-updated instances.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
12a1732a9fafde3d4180fde095ca57f35373771b 
  src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 
bd3196318b4fbd9f5fe5deb8294b5007f1ba93f0 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 a5ca21bf1e53cce1573d4512d4048d1a37ed50de 
  src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
d0deada3dc5335b2b6c2794256e8429ef9a4646c 

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


Testing
---


Thanks,

Bill Farner