Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 4:58 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Rebase correctly.


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
7aa061917c40903bd2f2582a4e928cdfbea81273 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
60d222f49e594fd7e5363da445fc57ddf8dc4aba 

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


Testing
---


Thanks,

Kevin Sweeney



Review Request 39572: Remove callable check.

2015-10-22 Thread Kevin Sweeney

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

Review request for Aurora, Joshua Cohen and Zameer Manji.


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


Repository: aurora


Description
---

Remove callable check.


Diffs
-

  src/main/python/apache/aurora/common/auth/auth_module_manager.py 
07a2730a0d4221cd989e85e40daf117a2da16cb4 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 4:27 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Remove bug id


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
7aa061917c40903bd2f2582a4e928cdfbea81273 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
60d222f49e594fd7e5363da445fc57ddf8dc4aba 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 4:26 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

rebase


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


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
7aa061917c40903bd2f2582a4e928cdfbea81273 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
60d222f49e594fd7e5363da445fc57ddf8dc4aba 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney


> On Oct. 22, 2015, 2:41 p.m., Kevin Sweeney wrote:
> > Elaborating - as far as I can see it the only sensible implementation of 
> > this feature would be for the client to set the version in the *request* 
> > and for the scheduler to check it and decide whether it supported the 
> > requested protocol version before processing it, and for the *scheduler* to 
> > return something like HTTP's `501 Not Implemented`. Otherwise there are no 
> > compatibility guarantees whatsoever. IMO `ServerInfo` should be much more 
> > like HTTP's `Server` header - additional information the server can provide 
> > about itself.

This fixes compatibility with the released 0.9.0 scheduler though, which is the 
subject of the linked ticket. As noted there's no reason to crash here. A 
scheduler change will not take effect until 0.10.0.


- Kevin


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


On Oct. 22, 2015, 2:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 2:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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


Elaborating - as far as I can see it the only sensible implementation of this 
feature would be for the client to set the version in the *request* and for the 
scheduler to check it and decide whether it supported the requested protocol 
version before processing it, and for the *scheduler* to return something like 
HTTP's `501 Not Implemented`. Otherwise there are no compatibility guarantees 
whatsoever. IMO `ServerInfo` should be much more like HTTP's `Server` header - 
additional information the server can provide about itself.

- Kevin Sweeney


On Oct. 22, 2015, 2:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 2:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 2:35 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Remove unused import.


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


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney


> On Oct. 22, 2015, 11:02 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, lines 313-315
> > <https://reviews.apache.org/r/39563/diff/1/?file=1103486#file1103486line313>
> >
> > This change suggests we are effectively dropping the support for this 
> > feature. I agree the design is flawed but probably not worth changing it 
> > now in view of REST API refactoring though. 
> > 
> > How about setting the version info in auth interceptor instead?

I plan to follow up with a patch to do that as well (it's slightly more 
involved) but I see no reason to keep this feature in place.


- Kevin


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


On Oct. 22, 2015, 10:47 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 10:47 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Kevin Sweeney


> On Oct. 22, 2015, 10:42 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 56-62
> > <https://reviews.apache.org/r/39532/diff/3/?file=1103469#file1103469line56>
> >
> > Why removing these fields now? This does not feel like a safe 
> > deprecation approach.

v0.9.0 schedulers ignore these fields, they just happen to call requireNonNull 
on this object. In thrift it is always fine to send extra fields that the 
deserializer is unaware of; they will simply be dropped on the floor.


- Kevin


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


On Oct. 22, 2015, 10:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39532/
> ---
> 
> (Updated Oct. 22, 2015, 10:36 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Always set SessionKey to empty on the client, as it's now ignored by the 
> scheduler.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f0d4ef824562093492a8f3c9efa2059908f4d98b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 5847ca88b0aeb828e7d03538725b3430ecd209ab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 53a3182896a9d385899e1f0274b2bfbe053076bb 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 3b14d888b52241927a1005a518516174e907d7eb 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 87935553d37db8f0a1d03d3c370cf717b5277d74 
> 
> Diff: https://reviews.apache.org/r/39532/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/:: 
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Kevin Sweeney

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


@reviewbot retry

- Kevin Sweeney


On Oct. 22, 2015, 10:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39532/
> ---
> 
> (Updated Oct. 22, 2015, 10:36 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Always set SessionKey to empty on the client, as it's now ignored by the 
> scheduler.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f0d4ef824562093492a8f3c9efa2059908f4d98b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 5847ca88b0aeb828e7d03538725b3430ecd209ab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 53a3182896a9d385899e1f0274b2bfbe053076bb 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 3b14d888b52241927a1005a518516174e907d7eb 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 87935553d37db8f0a1d03d3c370cf717b5277d74 
> 
> Diff: https://reviews.apache.org/r/39532/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/:: 
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

Review request for Aurora, Maxim Khutornenko and Zameer Manji.


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


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 10:36 a.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Also remove fields and document deprecation in api.thrift.


Repository: aurora


Description
---

Always set SessionKey to empty on the client, as it's now ignored by the 
scheduler.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f0d4ef824562093492a8f3c9efa2059908f4d98b 
  src/main/python/apache/aurora/client/api/__init__.py 
5847ca88b0aeb828e7d03538725b3430ecd209ab 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
  src/main/python/apache/aurora/common/auth/auth_module.py 
53a3182896a9d385899e1f0274b2bfbe053076bb 
  src/main/python/apache/aurora/common/auth/auth_module_manager.py 
73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
  src/test/python/apache/aurora/client/api/test_restarter.py 
3b14d888b52241927a1005a518516174e907d7eb 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
  src/test/python/apache/aurora/client/api/test_updater.py 
87935553d37db8f0a1d03d3c370cf717b5277d74 

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


Testing
---

./pants test.pytest --no-fast src/test/python/:: 

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


Thanks,

Kevin Sweeney



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-21 Thread Kevin Sweeney

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

(Updated Oct. 21, 2015, 5:30 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Sort imports to appease the bot.


Repository: aurora


Description
---

Always set SessionKey to empty on the client, as it's now ignored by the 
scheduler.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/__init__.py 
5847ca88b0aeb828e7d03538725b3430ecd209ab 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
  src/main/python/apache/aurora/common/auth/auth_module.py 
53a3182896a9d385899e1f0274b2bfbe053076bb 
  src/main/python/apache/aurora/common/auth/auth_module_manager.py 
73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
  src/test/python/apache/aurora/client/api/test_restarter.py 
3b14d888b52241927a1005a518516174e907d7eb 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
  src/test/python/apache/aurora/client/api/test_updater.py 
87935553d37db8f0a1d03d3c370cf717b5277d74 

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


Testing
---

./pants test.pytest --no-fast src/test/python/:: 

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


Thanks,

Kevin Sweeney



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-21 Thread Kevin Sweeney

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

(Updated Oct. 21, 2015, 4:59 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Add e2e test results.


Repository: aurora


Description
---

Always set SessionKey to empty on the client, as it's now ignored by the 
scheduler.


Diffs
-

  src/main/python/apache/aurora/client/api/__init__.py 
5847ca88b0aeb828e7d03538725b3430ecd209ab 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
  src/main/python/apache/aurora/common/auth/auth_module.py 
53a3182896a9d385899e1f0274b2bfbe053076bb 
  src/main/python/apache/aurora/common/auth/auth_module_manager.py 
73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
  src/test/python/apache/aurora/client/api/test_restarter.py 
3b14d888b52241927a1005a518516174e907d7eb 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
  src/test/python/apache/aurora/client/api/test_updater.py 
87935553d37db8f0a1d03d3c370cf717b5277d74 

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


Testing (updated)
---

./pants test.pytest --no-fast src/test/python/:: 

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


Thanks,

Kevin Sweeney



Review Request 39532: Always set SessionKey to empty in the client.

2015-10-21 Thread Kevin Sweeney

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

Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Repository: aurora


Description
---

Always set SessionKey to empty on the client, as it's now ignored by the 
scheduler.


Diffs
-

  src/main/python/apache/aurora/client/api/__init__.py 
5847ca88b0aeb828e7d03538725b3430ecd209ab 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
  src/main/python/apache/aurora/common/auth/auth_module.py 
53a3182896a9d385899e1f0274b2bfbe053076bb 
  src/main/python/apache/aurora/common/auth/auth_module_manager.py 
73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
  src/test/python/apache/aurora/client/api/test_restarter.py 
3b14d888b52241927a1005a518516174e907d7eb 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
  src/test/python/apache/aurora/client/api/test_updater.py 
87935553d37db8f0a1d03d3c370cf717b5277d74 

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


Testing
---

./pants test.pytest --no-fast src/test/python/:: 

Awaiting e2e test results.


Thanks,

Kevin Sweeney



Re: Review Request 39525: Ignore all SessionKeys.

2015-10-21 Thread Kevin Sweeney

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

(Updated Oct. 21, 2015, 3:44 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Appease the bot.


Repository: aurora


Description
---

This change ignores all SessionKeys. Since the scheduler was doing 
"requireNonNull" on SessionKey before the client needs to continue to set 
SessionKey to a non-null value for this release. In the next release the 
parameter can be dropped completely.

Apologies for the large diff, thankfully it's mostly red. Coverage in 
SchedulerThriftInterface is very high, most of the deleted tests deal with 
branches related to authentication or authorization failures, as these are now 
enforced in a different layer.


Diffs (updated)
-

  NEWS 7622f9db706c1764876f4174855343414dc59366 
  src/main/java/org/apache/aurora/auth/CapabilityValidator.java 
198cdf3a63195c6b125d6a2e5d726075fd41f809 
  src/main/java/org/apache/aurora/auth/SessionValidator.java 
b688a0f0f1a4549dc4fe81f5cc50af1167856e87 
  src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java 
476dd9a89ce317e3bd242fd70a55cc919aff8418 
  src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 
47f81aa4851c753779918dfeb137041073127cd4 
  src/main/java/org/apache/aurora/scheduler/app/MoreModules.java 
b79ccc849834c9a7f7ac654e4302f2ba4e13ff66 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
ee024a34f1fdde49791ad6573fe5c0ae3953a7c7 
  
src/main/java/org/apache/aurora/scheduler/http/api/GsonMessageBodyHandler.java 
2ec7da611db871749a79e2e93c1a2b988a1ca578 
  src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java 
3a29a628de8f1c1c3ed609b494656ac95db9efcf 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
304437e95dd1692b65bf5d0a6697ab62127e6833 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
e951f8e55353ff3c8d7d0e86016a2a004137c299 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
d110b21d4193eb37a10ee8ba2eca82d208496c38 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
 dd8d272751034ef7daa720e12dd4ed850532da39 
  src/main/java/org/apache/aurora/scheduler/thrift/auth/Requires.java 
9b3431899cddfc7f949fc8cb12f6e4739b4989d0 
  src/main/java/org/apache/aurora/scheduler/thrift/auth/ThriftAuthModule.java 
e1a21f632f2bee5834cbbde9d6902d71b6a090d9 
  src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 
c3c34656d781bc54282206caf1211d1385fd68c8 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
8af7c46e46d2269903118deb2a2a2b61bcc1d8d1 
  src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java 
0f8748d2981d48236f602778d2c1848b4c572445 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 f537f7a39496f69953e6d86cb15359fe3a705480 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
f06481bd237db0ab84850fe987d6e1fcb194f31b 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
b940e82644365fae613d577ba5b8779077f115d4 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 1bb3eae224de287d0ed83534f1c933ddd72298bb 

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


Testing
---

./graldew -Pq clean build


Thanks,

Kevin Sweeney



Re: Review Request 39525: Ignore all SessionKeys.

2015-10-21 Thread Kevin Sweeney

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

(Updated Oct. 21, 2015, 3:29 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Remove unused TypeAdapter to fix coverage warning.


Repository: aurora


Description
---

This change ignores all SessionKeys. Since the scheduler was doing 
"requireNonNull" on SessionKey before the client needs to continue to set 
SessionKey to a non-null value for this release. In the next release the 
parameter can be dropped completely.

Apologies for the large diff, thankfully it's mostly red. Coverage in 
SchedulerThriftInterface is very high, most of the deleted tests deal with 
branches related to authentication or authorization failures, as these are now 
enforced in a different layer.


Diffs (updated)
-

  NEWS 7622f9db706c1764876f4174855343414dc59366 
  src/main/java/org/apache/aurora/auth/CapabilityValidator.java 
198cdf3a63195c6b125d6a2e5d726075fd41f809 
  src/main/java/org/apache/aurora/auth/SessionValidator.java 
b688a0f0f1a4549dc4fe81f5cc50af1167856e87 
  src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java 
476dd9a89ce317e3bd242fd70a55cc919aff8418 
  src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 
47f81aa4851c753779918dfeb137041073127cd4 
  src/main/java/org/apache/aurora/scheduler/app/MoreModules.java 
b79ccc849834c9a7f7ac654e4302f2ba4e13ff66 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
ee024a34f1fdde49791ad6573fe5c0ae3953a7c7 
  
src/main/java/org/apache/aurora/scheduler/http/api/GsonMessageBodyHandler.java 
2ec7da611db871749a79e2e93c1a2b988a1ca578 
  src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java 
3a29a628de8f1c1c3ed609b494656ac95db9efcf 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
304437e95dd1692b65bf5d0a6697ab62127e6833 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
e951f8e55353ff3c8d7d0e86016a2a004137c299 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
d110b21d4193eb37a10ee8ba2eca82d208496c38 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
 dd8d272751034ef7daa720e12dd4ed850532da39 
  src/main/java/org/apache/aurora/scheduler/thrift/auth/Requires.java 
9b3431899cddfc7f949fc8cb12f6e4739b4989d0 
  src/main/java/org/apache/aurora/scheduler/thrift/auth/ThriftAuthModule.java 
e1a21f632f2bee5834cbbde9d6902d71b6a090d9 
  src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 
c3c34656d781bc54282206caf1211d1385fd68c8 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
8af7c46e46d2269903118deb2a2a2b61bcc1d8d1 
  src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java 
0f8748d2981d48236f602778d2c1848b4c572445 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 f537f7a39496f69953e6d86cb15359fe3a705480 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
f06481bd237db0ab84850fe987d6e1fcb194f31b 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
b940e82644365fae613d577ba5b8779077f115d4 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 1bb3eae224de287d0ed83534f1c933ddd72298bb 

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


Testing
---

./graldew -Pq clean build


Thanks,

Kevin Sweeney



Review Request 39525: Ignore all SessionKeys.

2015-10-21 Thread Kevin Sweeney

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

Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Repository: aurora


Description
---

This change ignores all SessionKeys. Since the scheduler was doing 
"requireNonNull" on SessionKey before the client needs to continue to set 
SessionKey to a non-null value for this release. In the next release the 
parameter can be dropped completely.

Apologies for the large diff, thankfully it's mostly red. Coverage in 
SchedulerThriftInterface is very high, most of the deleted tests deal with 
branches related to authentication or authorization failures, as these are now 
enforced in a different layer.


Diffs
-

  NEWS 7622f9db706c1764876f4174855343414dc59366 
  src/main/java/org/apache/aurora/auth/CapabilityValidator.java 
198cdf3a63195c6b125d6a2e5d726075fd41f809 
  src/main/java/org/apache/aurora/auth/SessionValidator.java 
b688a0f0f1a4549dc4fe81f5cc50af1167856e87 
  src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java 
476dd9a89ce317e3bd242fd70a55cc919aff8418 
  src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 
47f81aa4851c753779918dfeb137041073127cd4 
  src/main/java/org/apache/aurora/scheduler/app/MoreModules.java 
b79ccc849834c9a7f7ac654e4302f2ba4e13ff66 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
ee024a34f1fdde49791ad6573fe5c0ae3953a7c7 
  src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java 
3a29a628de8f1c1c3ed609b494656ac95db9efcf 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
304437e95dd1692b65bf5d0a6697ab62127e6833 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java 
e951f8e55353ff3c8d7d0e86016a2a004137c299 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
d110b21d4193eb37a10ee8ba2eca82d208496c38 
  
src/main/java/org/apache/aurora/scheduler/thrift/aop/UserCapabilityInterceptor.java
 dd8d272751034ef7daa720e12dd4ed850532da39 
  src/main/java/org/apache/aurora/scheduler/thrift/auth/Requires.java 
9b3431899cddfc7f949fc8cb12f6e4739b4989d0 
  src/main/java/org/apache/aurora/scheduler/thrift/auth/ThriftAuthModule.java 
e1a21f632f2bee5834cbbde9d6902d71b6a090d9 
  src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 
c3c34656d781bc54282206caf1211d1385fd68c8 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
8af7c46e46d2269903118deb2a2a2b61bcc1d8d1 
  src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java 
0f8748d2981d48236f602778d2c1848b4c572445 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 f537f7a39496f69953e6d86cb15359fe3a705480 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
f06481bd237db0ab84850fe987d6e1fcb194f31b 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java 
b940e82644365fae613d577ba5b8779077f115d4 
  
src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java
 1bb3eae224de287d0ed83534f1c933ddd72298bb 

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


Testing
---

./graldew -Pq clean build


Thanks,

Kevin Sweeney



Re: Review Request 39057: "aurora config read" command

2015-10-12 Thread Kevin Sweeney

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



src/main/python/apache/aurora/client/cli/config.py (lines 84 - 88)
<https://reviews.apache.org/r/39057/#comment159944>

Missing error handling - what if this is an invalid config / path?


- Kevin Sweeney


On Oct. 6, 2015, 11:43 a.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39057/
> ---
> 
> (Updated Oct. 6, 2015, 11:43 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-1504
> https://issues.apache.org/jira/browse/AURORA-1504
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> 1. Moves "metadata" into Job schema and out of AuroraConfig so that metadata 
> from bindings is saved/loaded from .json output.
> 2. Adds "config read" command that accepts same format as job create/inspect 
> etc.
> 
> This makes it simpler to build sensible deployment pipelines with binding 
> helpers.  Take an .aurora or .json config, "compile" it by making all 
> bindings concrete, check into a vcs, use for diff if necessary, 
> deploy/rollback from json without losing metadata.
> 
> 
> Diffs
> -
> 
>   .gitignore 6c37128b5d02b0e52eb467be3652a37b10d7bc06 
>   src/main/python/apache/aurora/client/cli/config.py 
> 73b556266183bf17463881b87cda107d07d79d71 
>   src/main/python/apache/aurora/config/__init__.py 
> 665e2cd08d4146e652b13bb82e04680315a1eebe 
>   src/main/python/apache/aurora/config/schema/base.py 
> 398f737bed9ef02ce4a5636896d6587bce26501e 
>   src/main/python/apache/aurora/config/thrift.py 
> b40a7fdfa63eaf18d9a85f62eec5aa717a7e7d91 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1bd745964b45b30014cabbef89949ac6221f148b 
> 
> Diff: https://reviews.apache.org/r/39057/diff/
> 
> 
> Testing
> ---
> 
> # build
> mba=aurora=; ./pants binary src/main/python/apache/aurora/client:aurora
> 
> # list configs
> mba=aurora=; dist/aurora.pex config list examples/jobs/hello_world.aurora 
> jobs=[devcluster/www-data/prod/hello]
> 
> # write to disk
> mba=aurora=; dist/aurora.pex config read devcluster/www-data/prod/hello 
> examples/jobs/hello_world.aurora > /tmp/hello_world.json
> 
> # edit file to change command, ram
> mba=aurora=; joe example/jobs/hello_world.aurora
> 
> # compare
> mba=aurora=; diff <(dist/aurora.pex config read 
> devcluster/www-data/prod/hello examples/jobs/hello_world.aurora) 
> /tmp/hello_world.json 
> 
> 44c44
> < "cmdline": "\nwhile true; do\n  echo hello 
> world with twice as much ram\n  sleep 10\ndone\n  ", 
> ---
> > "cmdline": "\nwhile true; do\n  echo hello 
> world\n  sleep 10\ndone\n  ", 
> 56c56
> < "ram": 268435456
> ---
> > "ram": 134217728
> 
> # submit
> mba=aurora=; aurora update start --read-json devcluster/www-data/prod/hello 
> /tmp/hello_world.json
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>



Re: Review Request 39073: Generalize plugin interface for aurora client.

2015-10-12 Thread Kevin Sweeney

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



src/main/python/apache/aurora/client/cli/__init__.py (lines 171 - 172)
<https://reviews.apache.org/r/39073/#comment159940>

Since this doesn't appear to be a documented SPI currently I don't think we 
should set a precedent by retaining backwards-compatibility here.



src/main/python/apache/aurora/client/cli/__init__.py (lines 255 - 261)
<https://reviews.apache.org/r/39073/#comment159941>

Please add test coverage and documentation for this functionality. A NEWS 
entry would be good form here as well.


- Kevin Sweeney


On Oct. 6, 2015, 10:29 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39073/
> ---
> 
> (Updated Oct. 6, 2015, 10:29 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Generalize plugin interface for client.
> 
> 1. Renames ConfigurationPlugin to just Plugin, since you can register nouns; 
> aliases the two together
> 2. Adds get_nouns method that allows you to register new top-level commands
> 3. Searches entry_points for plugins registered under 
> apache.aurora.client.cli.plugin
> 
> I have used this to implement a proof-of-concept 'aurora deploy' staged 
> rollout framework as well as basic S3 integration (will publish to 
> github.com/wickman/sacker separately.)
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> c1c54549fd8718275041b54e1e91b070dadc05da 
> 
> Diff: https://reviews.apache.org/r/39073/diff/
> 
> 
> Testing
> ---
> 
> Used it to create a sacker plugin for aurora client.
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>



Re: Review Request 39150: Converting to Range in ConfigGroup thrift.

2015-10-09 Thread Kevin Sweeney


> On Oct. 9, 2015, 10:12 a.m., Kevin Sweeney wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 351
> > <https://reviews.apache.org/r/39150/diff/2/?file=1093460#file1093460line351>
> >
> > Please rename this to deprecatedInstanceIds.
> 
> David McLaughlin wrote:
> -1 to this deprecation technique.
> 
> Maxim Khutornenko wrote:
> I thought we decided to not follow this approach and rely on 
> announcements and deprecation tickets instead?

Can you elaborate on the rationale behind your -1?

Renaming this field does not alter binary compatibility in any way and provides 
feedback to the API user that the field is slated for removal (in the form of a 
compiler or unit test break, similar to the `@Deprecated` annotation). A 
comment here is very likely to be missed. We have used this technique in the 
past.


- Kevin


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


On Oct. 8, 2015, 6:32 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39150/
> ---
> 
> (Updated Oct. 8, 2015, 6:32 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Addressing a long standing TODO. Also some cleanup of unused util functions.
> 
> 
> Diffs
> -
> 
>   NEWS 2edcea61fafea09b08faf03af4335ca82629d46d 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> a04e644453bfecde44ec7b51b53f42dc82e90c96 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5c1bdb4aeab285c46475a54bd13aeb780541fa08 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 13b5c222a0d7b9dd347990e6c09aac09ee566315 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  fd07365faa5fe516680600ed774d35c564948b9b 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 85ae8ce9e51d1785e32ba3f2e4f31f35ec6f178d 
>   src/main/resources/scheduler/assets/js/services.js 
> b7699fe91d79f9a8141c8368da91443684b6994b 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  64cbd5b58d9254bb741e20e47165732e52569f70 
> 
> Diff: https://reviews.apache.org/r/39150/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> Manual UI testing
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 39150: Converting to Range in ConfigGroup thrift.

2015-10-09 Thread Kevin Sweeney

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



api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 351)
<https://reviews.apache.org/r/39150/#comment159598>

Please rename this to deprecatedInstanceIds.


- Kevin Sweeney


On Oct. 8, 2015, 6:32 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39150/
> ---
> 
> (Updated Oct. 8, 2015, 6:32 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Addressing a long standing TODO. Also some cleanup of unused util functions.
> 
> 
> Diffs
> -
> 
>   NEWS 2edcea61fafea09b08faf03af4335ca82629d46d 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> a04e644453bfecde44ec7b51b53f42dc82e90c96 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5c1bdb4aeab285c46475a54bd13aeb780541fa08 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 13b5c222a0d7b9dd347990e6c09aac09ee566315 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  fd07365faa5fe516680600ed774d35c564948b9b 
>   src/main/resources/scheduler/assets/js/controllers.js 
> 85ae8ce9e51d1785e32ba3f2e4f31f35ec6f178d 
>   src/main/resources/scheduler/assets/js/services.js 
> b7699fe91d79f9a8141c8368da91443684b6994b 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  64cbd5b58d9254bb741e20e47165732e52569f70 
> 
> Diff: https://reviews.apache.org/r/39150/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> Manual UI testing
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37851: Use "Mesos Agent" instead of "Mesos Slave" in docs.

2015-10-09 Thread Kevin Sweeney

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

(Updated Oct. 9, 2015, 9:38 a.m.)


Review request for Aurora, Maxim Khutornenko and Bill Farner.


Changes
---

rebase


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


Repository: aurora


Description
---

Use "Mesos Agent" instead of "Mesos Slave" in docs.

There are a few referencess left - those that refer to the `mesos-slave` 
command line and those that document `clusters.json` attributes that use the 
term `slave`. Those can be replaced when upstream changes the binary name and 
as part of https://issues.apache.org/jira/browse/AURORA-1455.

Additionally there is inconsistent capitalization in the docs - I've taken to 
using `Mesos Agent` or `Agent` but this is not consistent everywhere, nor did I 
endeavor to fix it as that would explode the size of this diff.


Diffs (updated)
-

  docs/client-cluster-configuration.md 86bd17cf2ca84d0e467be18fd957ce1d6f35e37c 
  docs/client-commands.md 9977b492d25f0630ff39661097553113bbea6158 
  docs/configuration-reference.md ba0dc864a10726456373c23b508e04dd3b08a3ef 
  docs/configuration-tutorial.md bbb1684645b628df5fe8a45fcce6813e718157af 
  docs/deploying-aurora-scheduler.md 22a27f8a7a5b09eb7fdc4b91963f397dafe86c00 
  docs/developing-aurora-client.md b662b6f5c3667e195dd04ec7eccdce1a4c070972 
  docs/monitoring.md 3cb2a7910311ec4aa128d2811f956dfe6331472b 
  docs/sla.md a558e0098e73172801d9fad927f1f85d0476413f 
  docs/tutorial.md 812a5ccc6aa9acb06e139f2525cef10da5a2b1ad 
  docs/user-guide.md e608500499f46ea61d4a8607e4d1b2451f88996c 
  docs/vagrant.md 3bc201f17977dbc402eeac307500db5cc9d6bb27 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 37851: Use "Mesos Agent" instead of "Mesos Slave" in docs.

2015-10-09 Thread Kevin Sweeney

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


@ReviewBot retry

- Kevin Sweeney


On Aug. 27, 2015, 11:52 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37851/
> ---
> 
> (Updated Aug. 27, 2015, 11:52 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1451
> https://issues.apache.org/jira/browse/AURORA-1451
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use "Mesos Agent" instead of "Mesos Slave" in docs.
> 
> There are a few referencess left - those that refer to the `mesos-slave` 
> command line and those that document `clusters.json` attributes that use the 
> term `slave`. Those can be replaced when upstream changes the binary name and 
> as part of https://issues.apache.org/jira/browse/AURORA-1455.
> 
> Additionally there is inconsistent capitalization in the docs - I've taken to 
> using `Mesos Agent` or `Agent` but this is not consistent everywhere, nor did 
> I endeavor to fix it as that would explode the size of this diff.
> 
> 
> Diffs
> -
> 
>   docs/client-cluster-configuration.md 
> 86bd17cf2ca84d0e467be18fd957ce1d6f35e37c 
>   docs/client-commands.md f91e5df0c19104a961aa9955478bf2d37a2aa7b6 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/configuration-tutorial.md d6e7c352e16964e19a41340cb03a016620c17f7d 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   docs/developing-aurora-client.md b662b6f5c3667e195dd04ec7eccdce1a4c070972 
>   docs/monitoring.md 3cb2a7910311ec4aa128d2811f956dfe6331472b 
>   docs/sla.md a558e0098e73172801d9fad927f1f85d0476413f 
>   docs/tutorial.md 812a5ccc6aa9acb06e139f2525cef10da5a2b1ad 
>   docs/user-guide.md e608500499f46ea61d4a8607e4d1b2451f88996c 
>   docs/vagrant.md 3bc201f17977dbc402eeac307500db5cc9d6bb27 
> 
> Diff: https://reviews.apache.org/r/37851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39170: Fix NPE on accessing crons set at impossible dates

2015-10-09 Thread Kevin Sweeney

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



src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java (line 29)
<https://reviews.apache.org/r/39170/#comment159585>

Can you prefer java.util.Optional here?



src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
(line 273)
<https://reviews.apache.org/r/39170/#comment159586>

It seems like an empty string should be an invalid cron schedule here - can 
you use `isSetCronSchedule` here instead?


- Kevin Sweeney


On Oct. 9, 2015, 5:41 a.m., Brice Arnould wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39170/
> ---
> 
> (Updated Oct. 9, 2015, 5:41 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1385
> https://issues.apache.org/jira/browse/AURORA-1385
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is for AURORA-1385
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 043ba7e6858db28001dfb07ea0c2ddf274a1c755 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> 1ddc4e1946910de798f7f423dd1b19ed56dece15 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> 13b5c222a0d7b9dd347990e6c09aac09ee566315 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  64cbd5b58d9254bb741e20e47165732e52569f70 
> 
> Diff: https://reviews.apache.org/r/39170/diff/
> 
> 
> Testing
> ---
> 
> Added a specific unit test
> 
> 
> Thanks,
> 
> Brice Arnould
> 
>



Re: Review Request 39169: Fix minor inconsistencies in the storage documentation

2015-10-09 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 9, 2015, 5:59 a.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39169/
> ---
> 
> (Updated Oct. 9, 2015, 5:59 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix minor inconsistencies in the storage documentation.
> 
> 
> Diffs
> -
> 
>   docs/storage-config.md 4ec33a14d3361dfd7f5c07a1d01e31bb96243029 
> 
> Diff: https://reviews.apache.org/r/39169/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>



Re: Review Request 38975: AURORA-1512: Aurora rpm missing std output switch

2015-10-05 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 2, 2015, 7:12 p.m., Jake Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38975/
> ---
> 
> (Updated Oct. 2, 2015, 7:12 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-1512
> https://issues.apache.org/jira/browse/AURORA-1512
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> rpm not logging stdout as reported on IRC by abhis.
> 
> 
> Diffs
> -
> 
>   specs/rpm/SOURCES/aurora.init.sh ac4c438 
>   specs/rpm/SOURCES/thermos-observer.init.sh 65a2452 
> 
> Diff: https://reviews.apache.org/r/38975/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jake Farrell
> 
>



Re: Review Request 38906: Use Shiro to generate audit messages when available.

2015-09-30 Thread Kevin Sweeney

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

(Updated Sept. 30, 2015, 2:31 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Fix FindBugs and CheckStyle.


Repository: aurora


Description
---

This ensures that Shiro is always used to generate audit messages when 
available.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
d69bc3947c4a791cac97212cedf44250be273524 
  src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 ac89618332926d5a25fe7ab7da3b270b4d42c723 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
4bd8b12996cbfc7d75a994033551374b32909511 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney



Review Request 38906: Use Shiro to generate audit messages when available.

2015-09-30 Thread Kevin Sweeney

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

Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Repository: aurora


Description
---

This ensures that Shiro is always used to generate audit messages when 
available.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
d69bc3947c4a791cac97212cedf44250be273524 
  src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 ac89618332926d5a25fe7ab7da3b270b4d42c723 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
4bd8b12996cbfc7d75a994033551374b32909511 

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


Testing
---

./gradlew build


Thanks,

Kevin Sweeney



Re: Review Request 38332: Convert all of our servlet implementations to jax-rs endpoints.

2015-09-14 Thread Kevin Sweeney

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

Ship it!



commons/src/main/java/org/apache/aurora/common/net/http/handlers/AbortHandler.java
 (lines 64 - 65)
<https://reviews.apache.org/r/38332/#comment155525>

Will this ever appear to the client? Presumably abortListener shuts down 
the http server. quitquitquit appears to get around this by forking a new 
thread for the listener.



commons/src/main/java/org/apache/aurora/common/net/http/handlers/ContentionPrinter.java
 (line 77)
<https://reviews.apache.org/r/38332/#comment155526>

`String.join`



commons/src/main/java/org/apache/aurora/common/net/http/handlers/QuitHandler.java
 (lines 63 - 64)
<https://reviews.apache.org/r/38332/#comment155527>

same question - will this actually run?



commons/src/main/java/org/apache/aurora/common/net/http/handlers/TimeSeriesDataSource.java
 (line 98)
<https://reviews.apache.org/r/38332/#comment155530>

isn't the `@Produces` annotation enough here? afaik we don't need to 
explicitly serialize and deserialize json in jax-rs due to the json provider



commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsJsonHandler.java
 (line 70)
<https://reviews.apache.org/r/38332/#comment155533>

same question - do we stilll need to explicitly serialize to json?



commons/src/test/java/org/apache/aurora/common/net/http/handlers/VarsHandlerTest.java
 (line 74)
<https://reviews.apache.org/r/38332/#comment155534>

String.join



src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java (lines 
199 - 208)
<https://reviews.apache.org/r/38332/#comment155542>

This feels like a DRY violation to me - consider adding

`public static final String PATH = "/api"`

etc to each resource and referencing them in both the `@Path` annotation 
and here.


- Kevin Sweeney


On Sept. 13, 2015, 11:06 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38332/
> ---
> 
> (Updated Sept. 13, 2015, 11:06 a.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> There are a few positive outcomes here:
> 
> * our HTTP serving code is much more uniform
> * endpoints have less boilerplate code
> * endpoints are easier to test
> * ServletModule setup is simpler and uniform
> 
> 
> Diffs
> -
> 
>   buildSrc/src/main/groovy/org/apache/aurora/build/CoverageReportCheck.groovy 
> b996d4597492a76db0f2571db4e6c1ae9b4bcf33 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/AbortHandler.java
>  e97bd825752bf04cca391e2119b5e33c66a1cab5 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/AssetHandler.java
>  7a44f07e8514f32637d1832273cb4d9081a14031 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/ContentionPrinter.java
>  1f8c453c8e582da44849a8e922d974a0e054c638 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/HealthHandler.java
>  cc5ad4d5e15eaff78832c033e77b4ac0a532f6b3 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/LogConfig.java
>  5520fb6a83d37884f6cace995f3cc3313c3980c4 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/QuitHandler.java
>  4ce3c971b8e8ca696d7d5ea4e7d348237b836513 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/StringTemplateServlet.java
>  60e0abb7f7bac4c84ece5e007a4b3980fbd9a585 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/TextResponseHandler.java
>  23068eb4b8d9f1d848b4d007e14e7d32f3189ee1 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/ThreadStackPrinter.java
>  5dd88041faafc563c87f51807476aa142e4942d5 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/TimeSeriesDataSource.java
>  e87fe2c2a2d2ed609273298eb57085bcb155c3b2 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsHandler.java
>  bf04525115fa4045e792f2c45116e8d10addb24f 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsJsonHandler.java
>  e97ec6085c52a155ff9978c5121a5f98a8f93593 
>   
> commons/src/test/java/org/apache/aurora/common/net/http/handlers/AssetHandlerTest.java
>  740c42fedf668233b80f878728620a10ecf9b86f 
>   
> commons/src/test/java/org/apache/aurora/common/net/http/handlers/VarsHandlerTest.java
>  34f62fb9db2d723f

Re: Review Request 38014: Remove StartupRegistry.

2015-09-09 Thread Kevin Sweeney

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

Ship it!



commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
(line 51)
<https://reviews.apache.org/r/38014/#comment154613>

Looks like Modules.combine is redundant here.



commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
 (line 108)
<https://reviews.apache.org/r/38014/#comment154614>

Consider subclassing AbstractScheduledService instead


- Kevin Sweeney


On Sept. 8, 2015, 4:21 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38014/
> ---
> 
> (Updated Sept. 8, 2015, 4:21 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove StartupRegistry.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
> 78ed7d0aae50c976eba9045b17587258edc588b0 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupRegistry.java
>  997ee77dd8f6e02c2becddf98e16cbbbeec4cb4f 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupStage.java 
> 80509010f37894108881d40cad9d8ade8610f309 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/LifecycleModule.java
>  1b6bd086a5a434cb7e4b92ed9cfd252e2518ba66 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java
>  3959ce3d688dd50399185925d91f0014fc1c43f9 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepository.java
>  6928e48073d152915ca42b6f46236b21c0882086 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
>  387e379a9c84d663f2af23e4760754a023219860 
>   config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
>   src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
> 1077816b696c4d2e97aafa59900b6acf2adce064 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6892a70042e25fd672475517325b4e4b69a0adab 
>   src/main/java/org/apache/aurora/scheduler/app/LifecycleModule.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8047622e206c9827e5cd8e40152a278d495bd0ff 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  4af49d5dcb1925c4055f5ada8601f6fcab5d7d00 
>   src/test/java/org/apache/aurora/scheduler/async/AsyncModuleTest.java 
> d0faf13e3f1e14b6e70d230fea0fb484f2105873 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> cb38c3e047efac483445f43b941c7eea8862cc9c 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> ed8e8119ac3dc41c18316a1ca6e34c178916b09d 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
> 3f045ff38d672266ce2e2bb26f729b0ca4657e81 
>   src/test/java/org/apache/aurora/scheduler/reconciliation/KillRetryTest.java 
> 2f8fc10278e291fb28011b86bfa7c83905c6307c 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaModuleTest.java 
> 63716459a4e21aa035b683ad46ef0a31620cfd98 
> 
> Diff: https://reviews.apache.org/r/38014/diff/
> 
> 
> Testing
> ---
> 
> test suite
> ./gradlew run
> end-to-end tests
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 38086: Add test environments for RPMs and debs.

2015-09-09 Thread Kevin Sweeney

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

Ship it!


LGTM once comments are addressed

- Kevin Sweeney


On Sept. 2, 2015, 7:59 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38086/
> ---
> 
> (Updated Sept. 2, 2015, 7:59 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Add test environments for RPMs and debs.
> 
> 
> Diffs
> -
> 
>   .gitignore 849ddff3b7ec917b5f4563e9a6d3ea63ea512a70 
>   test/deb/ubuntu-trusty/README.md PRE-CREATION 
>   test/deb/ubuntu-trusty/Vagrantfile PRE-CREATION 
>   test/deb/ubuntu-trusty/provision.sh PRE-CREATION 
>   test/rpm/centos-7/README.md PRE-CREATION 
>   test/rpm/centos-7/Vagrantfile PRE-CREATION 
>   test/rpm/centos-7/provision.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 38086: Add test environments for RPMs and debs.

2015-09-09 Thread Kevin Sweeney

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



test/deb/ubuntu-trusty/Vagrantfile (lines 6 - 12)
<https://reviews.apache.org/r/38086/#comment154537>

mutli-vm boilerplate isn't needed

Can replace this block with

```ruby
config.vm.provider :virtualbox do |vb|
  vb.customize [...]
end
config.vm.provision "shell", path: "provision.sh"
```



test/deb/ubuntu-trusty/provision.sh (line 9)
<https://reviews.apache.org/r/38086/#comment154538>

https



test/rpm/centos-7/README.md (line 8)
<https://reviews.apache.org/r/38086/#comment154539>

https



test/rpm/centos-7/Vagrantfile (lines 6 - 11)
<https://reviews.apache.org/r/38086/#comment154544>

Drop multi-vm boilerplate



test/rpm/centos-7/provision.sh (lines 3 - 5)
<https://reviews.apache.org/r/38086/#comment154543>

yum should include these packages when we install the others, can this line 
be omitted?



test/rpm/centos-7/provision.sh (line 7)
<https://reviews.apache.org/r/38086/#comment154540>

https



test/rpm/centos-7/provision.sh (line 12)
<https://reviews.apache.org/r/38086/#comment154541>

https


- Kevin Sweeney


On Sept. 2, 2015, 7:59 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38086/
> ---
> 
> (Updated Sept. 2, 2015, 7:59 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Add test environments for RPMs and debs.
> 
> 
> Diffs
> -
> 
>   .gitignore 849ddff3b7ec917b5f4563e9a6d3ea63ea512a70 
>   test/deb/ubuntu-trusty/README.md PRE-CREATION 
>   test/deb/ubuntu-trusty/Vagrantfile PRE-CREATION 
>   test/deb/ubuntu-trusty/provision.sh PRE-CREATION 
>   test/rpm/centos-7/README.md PRE-CREATION 
>   test/rpm/centos-7/Vagrantfile PRE-CREATION 
>   test/rpm/centos-7/provision.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 38112: Alter thrift wrapper generator to use default primitive values and empty collections.

2015-09-08 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Sept. 8, 2015, 3:47 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38112/
> ---
> 
> (Updated Sept. 8, 2015, 3:47 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1476
> https://issues.apache.org/jira/browse/AURORA-1476
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Alter thrift wrapper generator to use default primitive values and empty 
> collections.
> 
> Reviewers - please see my self-review commentary in specific parts of the 
> patch.  The biggest shift with this change is that an impedance mismatch that 
> existed when inserting/fetching a record from the database is now lifted to 
> the thrift wrapper layer.
> 
> This means the following test is not guaranteed to pass:
> ```java
> TaskConfig original = new TaskConfig(...);
> assertEquals(original, ITaskConfig.build(original).newBuilder());  // won't 
> always pass
> ```
> 
> Most specifically, the wrapped/upwrapped `TaskConfig` will have null 
> collections replaced with empty ones, and will not honor set/unset flags for 
> primitives.  The best practice, therefore, should be to treat our wrapper 
> classes as the canonical form, and only prefer with the underlying thrift 
> types when absolutely necessary.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  a952797315a3421695748f09b9a6abb552cbb668 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 8787aeaa6655cfab1e0a6d5719f9e08a89df7631 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
> f1b167bacbfce4f753fc0bbb2b860e3024b9843f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  e12ad3e3868472ba84e379986bd1aa97bca42ffe 
>   src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py 
> b5f2bc9e54b525a6a782d8873c9112f6496cd3f2 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> b2ec13f40c12e5ee5663f4465734d6a80f3587cd 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  f3b62cc957186bc9673060830572bc1cc073ac49 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  5d8bd1b72927786df95c972df64d68c78f25dad0 
>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
> 08e1284ac1ef08b7649ed83df0a55be04cfeb88f 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  9213b88ab4ce5063ca0fb055851ae5632616155e 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> db60cd21d06d636505202bac7277a13dc24d46e6 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 295974a9f97e020dce11474d500a1bcd40d9f5d5 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
> 7ccc273204d20c84bbb576958e832b6f4a29f607 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java
>  0e0a847f46c4d1d833a3c610e8a5f752f368c0d5 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  3e78c097a7a9252ded8a4a7fc6609ecf5d61c5b5 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> e0110e7ebe631b7b66b2341cedc10490da00a2ab 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  4d4e752088f7dca99675cc66782ae046bbd516d6 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  4685aa157be77502ad0e4e648ad333ee286f3de5 
> 
> Diff: https://reviews.apache.org/r/38112/diff/
> 
> 
> Testing
> ---
> 
> End-to-end tests pass
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 38111: Disable mimetype guessing in the observer chroot browser.

2015-09-04 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Sept. 3, 2015, 3:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38111/
> ---
> 
> (Updated Sept. 3, 2015, 3:15 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1479
> https://issues.apache.org/jira/browse/AURORA-1479
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This disables the mimetype guessing of files served by the chroot browser and 
> sets it to the standard `application/octet-stream` mimetype. This prevents 
> browsers from trying to decompress gzipped files or other surprising 
> behaviour for users.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 1750f5bd0937f8ce411f976db488bf5d14930577 
> 
> Diff: https://reviews.apache.org/r/38111/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/observer::
> Manual inspection of Content-Type header via vagrant.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 38111: Disable mimetype guessing in the observer chroot browser.

2015-09-03 Thread Kevin Sweeney

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


Are you sure this works? AIUI it's Transfer-Encoding: gzip that we need to 
worry about, not content-encoding (which is part of the entity being served). 
Most web frameworks have a facility whereby they will locate pre-compressed 
assets, usually with a .gz suffix.

For example, if a directory contains `a.js` and `a.js.gz`, a client's request 
for a.js with the header `'accept-encoding: gzip'` will result in the server 
responding with the contents of `a.js.gz`, and the headers `transfer-encoding: 
gzip` and `content-type: application/javascript`. The client will generally 
decompress the result on the fly. OTOH, a request for `a.js.gz` should result 
in the unmodified file with `content-encoding: application/gzip`. See this 
stackoverflow post: [1]

[1] 
http://stackoverflow.com/questions/11641923/transfer-encoding-gzip-vs-content-encoding-gzip

- Kevin Sweeney


On Sept. 3, 2015, 3:15 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38111/
> ---
> 
> (Updated Sept. 3, 2015, 3:15 p.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Bugs: AURORA-1479
> https://issues.apache.org/jira/browse/AURORA-1479
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This disables the mimetype guessing of files served by the chroot browser and 
> sets it to the standard `application/octet-stream` mimetype. This prevents 
> browsers from trying to decompress gzipped files or other surprising 
> behaviour for users.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 1750f5bd0937f8ce411f976db488bf5d14930577 
> 
> Diff: https://reviews.apache.org/r/38111/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/observer::
> Manual inspection of Content-Type header via vagrant.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Review Request 38084: Builder should run createrepo

2015-09-02 Thread Kevin Sweeney

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

Review request for Aurora and Bill Farner.


Repository: aurora-packaging


Description
---

Builder should run createrepo


Diffs
-

  builder/rpm/centos-7/build.sh 912f7c788be648f8f3a11ec868a56d2994c62117 

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


Testing
---

Tested on 0.9.x branch


Thanks,

Kevin Sweeney



Re: Review Request 38080: Remove openjdk runtime requirement.

2015-09-02 Thread Kevin Sweeney


> On Sept. 2, 2015, 5:08 p.m., Bill Farner wrote:
> > Ship It!

procedural question - I see there's a long-running 0.9.x branch - should I 
merge this to master then merge this into that branch?


- Kevin


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


On Sept. 2, 2015, 4:36 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38080/
> ---
> 
> (Updated Sept. 2, 2015, 4:36 p.m.)
> 
> 
> Review request for Aurora, Steve Salevan and Bill Farner.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Remove openjdk runtime requirement.
> 
> This addresses one of my +0s.
> 
> 
> Diffs
> -
> 
>   specs/rpm/aurora.spec a559b42d73a4211891d8c7569ea7ab2a3a6993ae 
> 
> Diff: https://reviews.apache.org/r/38080/diff/
> 
> 
> Testing
> ---
> 
> Using a clean vagrant environment installed aurora, then installed the oracle 
> jdk, then uninstalled java-1.8.0-openjdk-headless (that aurora brought in 
> transitively). RPM didn't complain and when I restarted the scheduler it was 
> running under the new JVM.
> 
> ```
> [vagrant@localhost ~]$ curl -s http://localhost:8081/vars | grep 
> 'jvm_prop_java_vendor '
> jvm_prop_java_vendor Oracle Corporation
> [vagrant@localhost ~]$ 
> ```
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-09-02 Thread Kevin Sweeney


> On Sept. 2, 2015, 9:45 a.m., Bill Farner wrote:
> > examples/vagrant/executors-config.json, lines 4-7
> > 
> >
> > The code later converts this array into a single command string.  I 
> > suggest we just make this a string here (and save some code down the line).
> 
> Renan DelValle wrote:
> We can go ahead and do that. Kevin thought it should be the array way, 
> but I'm not sure how strongly he feels about it. The extra code isn't too bad 
> because we can use String.join() form Java 8. Either way, I'm fine with 
> either. The single string may eliminate any suspicions that we're breaking 
> something in a command when joining the array.

Bill, I prefer the array mode as it's more explicit for dealing with magic 
characters in shell commands (like " (){}$").

You can still explicitly do

```
["sh", "-c", "PATH=/opt/aurora/executor/bin:$PATH thermos"]
```

When you prefer the old form, but this form will not automatically expand 
environment variables or do other shell magic for you.

Renan - can you update CommandUtil to use the `arguments`[1] field instead of 
`shell` and `value` (or drop a TODO for a later patch)? Changing back to 
`String.join` defeats part of the purpose of the array form here.

[1] 
https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L408-L423


- Kevin


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


On Sept. 1, 2015, 7:35 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37818/
> ---
> 
> (Updated Sept. 1, 2015, 7:35 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is the first stage in a series of patches to create support for custom 
> executors. In an effort to expedite the review process, I have decided to 
> break down my patch into multiple pieces that when/if commited won't break 
> the trunk.
> 
> This patch includes the ability to load configuration from a JSON file. A 
> JSON example file is included in examples/vagrant/executors-config.json
> 
> Command line arguments have been eliminated and moved over to the JSON file. 
> GSON is leveraged and does most of the work with the aid of a few custom 
> deserializers that were needed. 
> 
> Note that right now a global container mount that does not follow 
> specification will cause the scheduler to detect the error an exit early. It 
> is up for discussion if this is the desired behavior or if we should just 
> ignore said mount.
> 
> 
> Diffs
> -
> 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 4f43892723db4744db205ea7dd107e9e9ce9d5db 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> e909451892f117e9e6eb80994079661827a0914c 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> c210c0db07bb1f4b3f76668178dcd7e2de56a4ac 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 197184b6edc0768d677636341b5737f262abdf7d 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8047622e206c9827e5cd8e40152a278d495bd0ff 
>   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
> aa5ce8b2f14c7dbd0eae120018ee41387c26059f 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> f6ba2c40aea555d3e0ab774218bfe08d7e1c984b 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 6fad3344042dc6a75cdf74ce79d388fcd4fc9861 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 1a25924d789295c5950947f5e302e1d1fbec68f2 
>   src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java 
> cd0295780d41bc4e914583f195b37eaed28a46dc 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoaderTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> dddf7952d3f0e508cd736d5fb95e573267708d43 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> d0987251b058988fcbfab16c1b138c37e0c5b8c6 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/executor-settings-thermos-no-observer.json
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/multiple-executor-example.json
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/no-val

Review Request 38080: Remove openjdk runtime requirement.

2015-09-02 Thread Kevin Sweeney

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

Review request for Aurora, Steve Salevan and Bill Farner.


Repository: aurora-packaging


Description
---

Remove openjdk runtime requirement.

This addresses one of my +0s.


Diffs
-

  specs/rpm/aurora.spec a559b42d73a4211891d8c7569ea7ab2a3a6993ae 

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


Testing
---

Using a clean vagrant environment installed aurora, then installed the oracle 
jdk, then uninstalled java-1.8.0-openjdk-headless (that aurora brought in 
transitively). RPM didn't complain and when I restarted the scheduler it was 
running under the new JVM.

```
[vagrant@localhost ~]$ curl -s http://localhost:8081/vars | grep 
'jvm_prop_java_vendor '
jvm_prop_java_vendor Oracle Corporation
[vagrant@localhost ~]$ 
```


Thanks,

Kevin Sweeney



Re: Review Request 38014: Remove StartupRegistry.

2015-09-01 Thread Kevin Sweeney

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



src/main/java/org/apache/aurora/GuavaUtils.java (lines 114 - 124)
<https://reviews.apache.org/r/38014/#comment153167>

Rather than change the purpose of this class from a dumb adapter that makes 
`ServiceManager` mockable, consider adding a `Listener` to `ServiceManager` and 
overriding `stopped`. This fits the canonical example of ServiceManager API 
usage.



src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
<https://reviews.apache.org/r/38014/#comment153172>

Is this shutdown timeout preserved anywhere? Seems this should move to a 
`Listener` on the below providers.


- Kevin Sweeney


On Sept. 1, 2015, 11:59 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38014/
> ---
> 
> (Updated Sept. 1, 2015, 11:59 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove StartupRegistry.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/application/AppLauncher.java 
> 78ed7d0aae50c976eba9045b17587258edc588b0 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupRegistry.java
>  997ee77dd8f6e02c2becddf98e16cbbbeec4cb4f 
>   
> commons/src/main/java/org/apache/aurora/common/application/StartupStage.java 
> 80509010f37894108881d40cad9d8ade8610f309 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/LifecycleModule.java
>  1b6bd086a5a434cb7e4b92ed9cfd252e2518ba66 
>   
> commons/src/main/java/org/apache/aurora/common/application/modules/StatsModule.java
>  3959ce3d688dd50399185925d91f0014fc1c43f9 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepository.java
>  6928e48073d152915ca42b6f46236b21c0882086 
>   
> commons/src/main/java/org/apache/aurora/common/stats/TimeSeriesRepositoryImpl.java
>  387e379a9c84d663f2af23e4760754a023219860 
>   config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> e3e90e3e43744463a2f00552f0041d0d1945bd57 
>   src/main/java/org/apache/aurora/scheduler/SchedulerServicesModule.java 
> 1077816b696c4d2e97aafa59900b6acf2adce064 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 6892a70042e25fd672475517325b4e4b69a0adab 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 8047622e206c9827e5cd8e40152a278d495bd0ff 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 1a25924d789295c5950947f5e302e1d1fbec68f2 
>   
> src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java
>  4af49d5dcb1925c4055f5ada8601f6fcab5d7d00 
>   src/test/java/org/apache/aurora/scheduler/events/PubsubEventModuleTest.java 
> cb38c3e047efac483445f43b941c7eea8862cc9c 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> ed8e8119ac3dc41c18316a1ca6e34c178916b09d 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
> 3f045ff38d672266ce2e2bb26f729b0ca4657e81 
> 
> Diff: https://reviews.apache.org/r/38014/diff/
> 
> 
> Testing
> ---
> 
> test suite
> ./gradlew run
> end-to-end tests
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37938: Upgrade Shiro to 1.2.4.

2015-08-31 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 30, 2015, 9:49 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37938/
> ---
> 
> (Updated Aug. 30, 2015, 9:49 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1186
> https://issues.apache.org/jira/browse/AURORA-1186
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Upgrade Shiro to 1.2.4.
> 
> 
> Diffs
> -
> 
>   build.gradle ce99f3aeb846a3184b9ea82d4e2e84eb31bc0119 
> 
> Diff: https://reviews.apache.org/r/37938/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>



Re: Review Request 37924: Deb: move clusters.json from aurora-executor to aurora-tools package.

2015-08-29 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 29, 2015, 9:38 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37924/
> ---
> 
> (Updated Aug. 29, 2015, 9:38 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Deb: move clusters.json from aurora-executor to aurora-tools package.
> 
> 
> Diffs
> -
> 
>   specs/debian/aurora-executor.install 
> 5d0d1f75b56ff0c8064fe02d516a6e257168 
>   specs/debian/aurora-tools.install ac8d032d3c6f86ba8e9f73ea7bc712ebde7531f8 
> 
> Diff: https://reviews.apache.org/r/37924/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37862: Rpm: remove daemonize dep for rhel >=7.

2015-08-27 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 27, 2015, 4:08 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37862/
> ---
> 
> (Updated Aug. 27, 2015, 4:08 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> This appears to have been a mistake in the original spec.  This patch makes 
> the thermos block match the scheduler block.
> 
> 
> Diffs
> -
> 
>   specs/rpm/aurora.spec 3e7b8bef2272d935f3efb3e3ea6e4ac510026b7d 
> 
> Diff: https://reviews.apache.org/r/37862/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37861: Add a convenience to build all artifacts.

2015-08-27 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 27, 2015, 4:05 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37861/
> ---
> 
> (Updated Aug. 27, 2015, 4:05 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> This pulls in some complexity from the jenkins job, and is generally useful 
> when iterating on the tooling.
> 
> 
> Diffs
> -
> 
>   build-artifact.sh bb24b3619c76e80317ad65386f81ab70a44105bc 
> 
> Diff: https://reviews.apache.org/r/37861/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37861: Add a convenience to build all artifacts.

2015-08-27 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 27, 2015, 4:05 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37861/
> ---
> 
> (Updated Aug. 27, 2015, 4:05 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> This pulls in some complexity from the jenkins job, and is generally useful 
> when iterating on the tooling.
> 
> 
> Diffs
> -
> 
>   build-artifact.sh bb24b3619c76e80317ad65386f81ab70a44105bc 
> 
> Diff: https://reviews.apache.org/r/37861/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37860: Rpm: fix install dep on daemonize, be permissive with newer mesos versions.

2015-08-27 Thread Kevin Sweeney

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



specs/rpm/aurora.spec (line 100)
<https://reviews.apache.org/r/37860/#comment152429>

Can you explain the rationale behind this change - unpinned mesos runs the 
risk of not working with the executor it was compiled against. Much as we might 
wish that this will work, we don't know that it will and it's much easier to 
debug an error from yum than a runtime linker issue. If a user is convinced 
that we're wrong here they can always override with `--nodeps`.


- Kevin Sweeney


On Aug. 27, 2015, 3:50 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37860/
> ---
> 
> (Updated Aug. 27, 2015, 3:50 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Rpm: fix install dep on daemonize, be permissive with newer mesos versions.
> 
> 
> Diffs
> -
> 
>   specs/rpm/aurora.spec 3e7b8bef2272d935f3efb3e3ea6e4ac510026b7d 
> 
> Diff: https://reviews.apache.org/r/37860/diff/
> 
> 
> Testing
> ---
> 
> Successfully ran what jenkins runs:
> ```
> ./build-artifact.sh builder/rpm/centos-7 ~/snapshot.tar.gz 
> 0.10.0snapshot.2015.08.27
> ```
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37847: Change the UI to refer to Mesos Agents instead of Mesos Slaves.

2015-08-27 Thread Kevin Sweeney

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

(Updated Aug. 27, 2015, 3:13 p.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Add Agents to legacy_untested_classes.txt


Bugs: AURORA-1449 and AURORA-1450
https://issues.apache.org/jira/browse/AURORA-1449
https://issues.apache.org/jira/browse/AURORA-1450


Repository: aurora


Description
---

Change the UI to refer to Mesos Agents instead of Mesos Slaves.

This change also moves the /slaves endpoint to /agents and adds a compatibility 
redirect.


Diffs (updated)
-

  config/legacy_untested_classes.txt 4bae43a43d00f71456f37f001fcd21ce6a2fb841 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
05af127a91b6c0e8db089cb828c38e556b07d45f 
  src/main/java/org/apache/aurora/scheduler/http/Slaves.java 
a9fef25595d616f0fbbb7c5ad6de39184a60002a 
  src/main/resources/org/apache/aurora/scheduler/http/slaves.st 
854a9d4a3dc0be6bdf0ea0ad6194ffce48039919 
  src/main/resources/org/apache/aurora/scheduler/http/utilization.st 
4a8a07222d65681e77c90566be9dce9283c7be65 
  src/main/resources/scheduler/assets/index.html 
d0bd897524a4c763d11a601898b663fec672e979 
  src/main/resources/scheduler/assets/js/filters.js 
bf6aef86fd50ba38b64f5d9f481d6915c38881f7 

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


Testing
---

Tested /slaves and /agents in vagrant.


Thanks,

Kevin Sweeney



Review Request 37859: Rename slave to agent in src/main/python.

2015-08-27 Thread Kevin Sweeney

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

Review request for Aurora, Bill Farner and Zameer Manji.


Bugs: AURORA-1454 and AURORA-1455
https://issues.apache.org/jira/browse/AURORA-1454
https://issues.apache.org/jira/browse/AURORA-1455


Repository: aurora


Description
---

Use "Mesos Agent" instead of "Mesos Slave" in src/main/python.

This changes the name in all places where the Python code controls the use of 
the name (comments, pydoc, private variable names). It doesn't change the 
reference in places it doesn't control (`clusters.json`, `api.thrift`, 
`mesos.proto`, and the `Executor` interface from `mesos.interface`).

For ease of review and confidence, I intend to change `src/test/python` in a 
follow-up review.


Diffs
-

  src/main/python/apache/aurora/client/api/command_runner.py 
c7238e274e53138187c2fe6fe5f14b7ae5f43ba2 
  src/main/python/apache/aurora/client/base.py 
487f8e73fa978b150f2e5b60e0f64c71ce783c12 
  src/main/python/apache/aurora/client/cli/task.py 
d1f2568ac0afdd95c65523fde41f0dd16670a7a8 
  src/main/python/apache/aurora/executor/aurora_executor.py 
96d7a336deebc4c95405d237df0ee11e7e055aa2 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
b3e8bf1e924999306e0b8a1314273b22c51028e7 
  src/main/python/apache/aurora/executor/common/announcer.py 
dda76f018f472d7d8228459eb89f4c5daf9df26d 
  src/main/python/apache/aurora/executor/common/executor_detector.py 
a07bfc34caa5f86153ace8184b061e253c39e92e 
  src/main/python/apache/aurora/executor/executor_base.py 
1db97cc8c12752d4eca339a7680ba963a66ffbce 

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


Testing
---


Thanks,

Kevin Sweeney



Review Request 37855: Rename slave to agent in scheduler src/main/java.

2015-08-27 Thread Kevin Sweeney
age/mem/MemTaskStore.java 
072fe459e1422d6d59499b17440232ab112254ba 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
3d89e43659750de63d7588f8574e7a350caea04b 
  src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 
de4fbbcc929a80cdbd3b6593ad4fedce18a88057 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java 
a1ac922d471013779710e02c0c9ca9f84b506807 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
 9213b88ab4ce5063ca0fb055851ae5632616155e 
  
src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
 1308a1c9884e7759e6139787710b367bbf9cd4e9 
  
src/test/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImplTest.java 
13c9f47941eb8ce6c79f3f1a7c530150e3e6fae2 
  
src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
 092df8cdb7901e045682d0f252109a08afc50c01 
  src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
bc3f364a8d49c0b8efb1164efedd6e0a8e9182c8 
  
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 
4d4e752088f7dca99675cc66782ae046bbd516d6 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney



Re: Review Request 37847: Change the UI to refer to Mesos Agents instead of Mesos Slaves.

2015-08-27 Thread Kevin Sweeney

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

(Updated Aug. 27, 2015, 11:55 a.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

rebase


Bugs: AURORA-1449 and AURORA-1450
https://issues.apache.org/jira/browse/AURORA-1449
https://issues.apache.org/jira/browse/AURORA-1450


Repository: aurora


Description
---

Change the UI to refer to Mesos Agents instead of Mesos Slaves.

This change also moves the /slaves endpoint to /agents and adds a compatibility 
redirect.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
05af127a91b6c0e8db089cb828c38e556b07d45f 
  src/main/java/org/apache/aurora/scheduler/http/Slaves.java 
a9fef25595d616f0fbbb7c5ad6de39184a60002a 
  src/main/resources/org/apache/aurora/scheduler/http/slaves.st 
854a9d4a3dc0be6bdf0ea0ad6194ffce48039919 
  src/main/resources/org/apache/aurora/scheduler/http/utilization.st 
4a8a07222d65681e77c90566be9dce9283c7be65 
  src/main/resources/scheduler/assets/index.html 
d0bd897524a4c763d11a601898b663fec672e979 
  src/main/resources/scheduler/assets/js/filters.js 
bf6aef86fd50ba38b64f5d9f481d6915c38881f7 

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


Testing
---

Tested /slaves and /agents in vagrant.


Thanks,

Kevin Sweeney



Review Request 37851: Use "Mesos Agent" instead of "Mesos Slave" in docs.

2015-08-27 Thread Kevin Sweeney

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

Review request for Aurora, Maxim Khutornenko and Bill Farner.


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


Repository: aurora


Description
---

Use "Mesos Agent" instead of "Mesos Slave" in docs.

There are a few referencess left - those that refer to the `mesos-slave` 
command line and those that document `clusters.json` attributes that use the 
term `slave`. Those can be replaced when upstream changes the binary name and 
as part of https://issues.apache.org/jira/browse/AURORA-1455.

Additionally there is inconsistent capitalization in the docs - I've taken to 
using `Mesos Agent` or `Agent` but this is not consistent everywhere, nor did I 
endeavor to fix it as that would explode the size of this diff.


Diffs
-

  docs/client-cluster-configuration.md 86bd17cf2ca84d0e467be18fd957ce1d6f35e37c 
  docs/client-commands.md f91e5df0c19104a961aa9955478bf2d37a2aa7b6 
  docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
  docs/configuration-tutorial.md d6e7c352e16964e19a41340cb03a016620c17f7d 
  docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
  docs/developing-aurora-client.md b662b6f5c3667e195dd04ec7eccdce1a4c070972 
  docs/monitoring.md 3cb2a7910311ec4aa128d2811f956dfe6331472b 
  docs/sla.md a558e0098e73172801d9fad927f1f85d0476413f 
  docs/tutorial.md 812a5ccc6aa9acb06e139f2525cef10da5a2b1ad 
  docs/user-guide.md e608500499f46ea61d4a8607e4d1b2451f88996c 
  docs/vagrant.md 3bc201f17977dbc402eeac307500db5cc9d6bb27 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 37825: Adding minimal implementation of the external tier config.

2015-08-27 Thread Kevin Sweeney


> On Aug. 27, 2015, 10:23 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 70
> > 
> >
> > +@CanRead

That will cause loading to fail when the argument is null.


- Kevin


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


On Aug. 26, 2015, 6:07 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37825/
> ---
> 
> (Updated Aug. 26, 2015, 6:07 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1437
> https://issues.apache.org/jira/browse/AURORA-1437
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The external config file is optional for now as tiers are not fully defined 
> yet.
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> d4bc6b9bb10c982fb6a2458d7fdd12bddbe98eca 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 652afece17a7eb09b0ca68066707b1b8fbf024f0 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> eb0207114e1b93968cb65832f154a9cd3bc3232e 
>   src/test/resources/org/apache/aurora/scheduler/tiers-example.json 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37825/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37847: Change the UI to refer to Mesos Agents instead of Mesos Slaves.

2015-08-27 Thread Kevin Sweeney

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

(Updated Aug. 27, 2015, 11:01 a.m.)


Review request for Aurora, Joshua Cohen and Bill Farner.


Changes
---

Add bug IDs


Bugs: AURORA-1449 and AURORA-1450
https://issues.apache.org/jira/browse/AURORA-1449
https://issues.apache.org/jira/browse/AURORA-1450


Repository: aurora


Description
---

Change the UI to refer to Mesos Agents instead of Mesos Slaves.

This change also moves the /slaves endpoint to /agents and adds a compatibility 
redirect.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
06ee83335bde7053227de0bd516ca8ce24f8ce55 
  src/main/java/org/apache/aurora/scheduler/http/Slaves.java 
b64e18c4d5278e6cdb2e480043a45e6ec4f87484 
  src/main/resources/org/apache/aurora/scheduler/http/slaves.st 
854a9d4a3dc0be6bdf0ea0ad6194ffce48039919 
  src/main/resources/org/apache/aurora/scheduler/http/utilization.st 
4a8a07222d65681e77c90566be9dce9283c7be65 
  src/main/resources/scheduler/assets/index.html 
d0bd897524a4c763d11a601898b663fec672e979 
  src/main/resources/scheduler/assets/js/filters.js 
bf6aef86fd50ba38b64f5d9f481d6915c38881f7 

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


Testing
---

Tested /slaves and /agents in vagrant.


Thanks,

Kevin Sweeney



Re: Review Request 37847: Change the UI to refer to Mesos Agents instead of Mesos Slaves.

2015-08-27 Thread Kevin Sweeney


> On Aug. 27, 2015, 10:46 a.m., Maxim Khutornenko wrote:
> > Should this be a part of an epic to rename all internal occurences? Quick 
> > check shows up plenty of mentiones in docs and public (e.g. thrift) APIs. I 
> > think consistency is important and we should not lose track of other work 
> > in this area.

This change was the result of a grep for the lowest-hanging fruit, but I 
restricted it to the `src/main` folder and didn't include `docs`. I can 
definitely include `docs` in a follow-up patch. Good call on the Epic - filed 
https://issues.apache.org/jira/browse/AURORA-1448


- Kevin


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


On Aug. 27, 2015, 10:37 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37847/
> ---
> 
> (Updated Aug. 27, 2015, 10:37 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Change the UI to refer to Mesos Agents instead of Mesos Slaves.
> 
> This change also moves the /slaves endpoint to /agents and adds a 
> compatibility redirect.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 06ee83335bde7053227de0bd516ca8ce24f8ce55 
>   src/main/java/org/apache/aurora/scheduler/http/Slaves.java 
> b64e18c4d5278e6cdb2e480043a45e6ec4f87484 
>   src/main/resources/org/apache/aurora/scheduler/http/slaves.st 
> 854a9d4a3dc0be6bdf0ea0ad6194ffce48039919 
>   src/main/resources/org/apache/aurora/scheduler/http/utilization.st 
> 4a8a07222d65681e77c90566be9dce9283c7be65 
>   src/main/resources/scheduler/assets/index.html 
> d0bd897524a4c763d11a601898b663fec672e979 
>   src/main/resources/scheduler/assets/js/filters.js 
> bf6aef86fd50ba38b64f5d9f481d6915c38881f7 
> 
> Diff: https://reviews.apache.org/r/37847/diff/
> 
> 
> Testing
> ---
> 
> Tested /slaves and /agents in vagrant.
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Review Request 37847: Change the UI to refer to Mesos Agents instead of Mesos Slaves.

2015-08-27 Thread Kevin Sweeney

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

Review request for Aurora, Joshua Cohen and Bill Farner.


Repository: aurora


Description
---

Change the UI to refer to Mesos Agents instead of Mesos Slaves.

This change also moves the /slaves endpoint to /agents and adds a compatibility 
redirect.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
06ee83335bde7053227de0bd516ca8ce24f8ce55 
  src/main/java/org/apache/aurora/scheduler/http/Slaves.java 
b64e18c4d5278e6cdb2e480043a45e6ec4f87484 
  src/main/resources/org/apache/aurora/scheduler/http/slaves.st 
854a9d4a3dc0be6bdf0ea0ad6194ffce48039919 
  src/main/resources/org/apache/aurora/scheduler/http/utilization.st 
4a8a07222d65681e77c90566be9dce9283c7be65 
  src/main/resources/scheduler/assets/index.html 
d0bd897524a4c763d11a601898b663fec672e979 
  src/main/resources/scheduler/assets/js/filters.js 
bf6aef86fd50ba38b64f5d9f481d6915c38881f7 

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


Testing
---

Tested /slaves and /agents in vagrant.


Thanks,

Kevin Sweeney



Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-27 Thread Kevin Sweeney


> On Aug. 26, 2015, 3:27 p.m., Kevin Sweeney wrote:
> > examples/vagrant/executors-config-new.json, line 18
> > <https://reviews.apache.org/r/37818/diff/1/?file=1055421#file1055421line18>
> >
> > this isn't a global property - can it be pushed into a custom 
> > configuration object?
> 
> Renan DelValle wrote:
> We can, but this change causes a ripple effect. If we push this into a 
> custom schema, we have to provide a way of unpacking custom configuration 
> object into useful mesos data fields or information usefol to the executor. 
> We need to come to a decision on the best way of doing this if we go for it. 
> Tangentially, as far as I can tell this is only used when building container 
> volumes.

As this is greenfield code I'm okay with a bit of a ripple - I'd much rather 
cluster operators have to write this config file once than change it as our 
design for it evolves.


> On Aug. 26, 2015, 3:27 p.m., Kevin Sweeney wrote:
> > examples/vagrant/executors-config-new.json, line 19
> > <https://reviews.apache.org/r/37818/diff/1/?file=1055421#file1055421line19>
> >
> > It would be ergnomically nice to default this to an empty list if it's 
> > left unset.
> 
> Renan DelValle wrote:
> I'll see if there's a way of doing that. If you come across anything, let 
> me know.

A bit of Googling suggests that setting defaults in a default constructor works 
with GSON here. In fact it seems that just about any way you can think of doing 
it GSON supports defaults (confirmed myself here: 
https://gist.github.com/kevints/d1e26514468863e8c088).

Setting defaults in an initializer works too - I suggest following the pattern 
in `ImmutableDefaultTest`, or `ImmutableInitializeOverrideTest` if you have a 
`@VisibleForTesting` constructor.


- Kevin


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


On Aug. 26, 2015, 3:19 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37818/
> ---
> 
> (Updated Aug. 26, 2015, 3:19 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is the first stage in a series of patches to create support for custom 
> executors. In an effort to expedite the review process, I have decided to 
> break down my patch into multiple pieces that when/if commited won't break 
> the trunk.
> 
> This patch includes the ability to load configuration from a JSON file. A 
> JSON example file is included in examples/vagrant/executors-config.json
> 
> Command line arguments have been eliminated and moved over to the JSON file. 
> GSON is leveraged and does most of the work with the aid of a few custom 
> deserializers that were needed. 
> 
> Note that right now a global container mount that does not follow 
> specification will cause the scheduler to detect the error an exit early. It 
> is up for discussion if this is the desired behavior or if we should just 
> ignore said mount.
> 
> 
> Diffs
> -
> 
>   examples/vagrant/executors-config-new.json PRE-CREATION 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0c440b5cd5b939872c1ee05d048bf739bfa977cb 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoaderTest.java
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/executor-settings-thermos-no-observer.json
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/thermos-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37818/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh: directory sandbox failed but it may be a 
> flaky test
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>



Re: Review Request 37818: Moved executor settings configuration to loadable JSON

2015-08-26 Thread Kevin Sweeney

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


Quick (incomplete) review on the design of the config object


examples/vagrant/executors-config-new.json (lines 13 - 15)
<https://reviews.apache.org/r/37818/#comment152193>

do these have defaults that we can omit? also I don't think we want to 
cache in the vagrant environment as we want changes to be reflected in new 
tasks immediately when developing.



examples/vagrant/executors-config-new.json (line 14)
<https://reviews.apache.org/r/37818/#comment152195>

don't extract this



examples/vagrant/executors-config-new.json (line 18)
<https://reviews.apache.org/r/37818/#comment152194>

this isn't a global property - can it be pushed into a custom configuration 
object?



examples/vagrant/executors-config-new.json (line 19)
<https://reviews.apache.org/r/37818/#comment152192>

It would be ergnomically nice to default this to an empty list if it's left 
unset.



examples/vagrant/executors-config.json (line 1)
<https://reviews.apache.org/r/37818/#comment152191>

delete this file? It looks like it has been superceded by the new format 
below



src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java (line 114)
<https://reviews.apache.org/r/37818/#comment152196>

    drop redundant use of `this`, here and below.


- Kevin Sweeney


On Aug. 26, 2015, 3:19 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37818/
> ---
> 
> (Updated Aug. 26, 2015, 3:19 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is the first stage in a series of patches to create support for custom 
> executors. In an effort to expedite the review process, I have decided to 
> break down my patch into multiple pieces that when/if commited won't break 
> the trunk.
> 
> This patch includes the ability to load configuration from a JSON file. A 
> JSON example file is included in examples/vagrant/executors-config.json
> 
> Command line arguments have been eliminated and moved over to the JSON file. 
> GSON is leveraged and does most of the work with the aid of a few custom 
> deserializers that were needed. 
> 
> Note that right now a global container mount that does not follow 
> specification will cause the scheduler to detect the error an exit early. It 
> is up for discussion if this is the desired behavior or if we should just 
> ignore said mount.
> 
> 
> Diffs
> -
> 
>   examples/vagrant/executors-config-new.json PRE-CREATION 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 0c440b5cd5b939872c1ee05d048bf739bfa977cb 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoaderTest.java
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/executor-settings-thermos-no-observer.json
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/configuration/thermos-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37818/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh: directory sandbox failed but it may be a 
> flaky test
> bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>



Re: Review Request 37789: Refer to shared task_configs table for job updates.

2015-08-26 Thread Kevin Sweeney

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

Ship it!


Ship It!


src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
(line 160)
<https://reviews.apache.org/r/37789/#comment152067>

Inline this method reference.


- Kevin Sweeney


On Aug. 25, 2015, 11:41 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37789/
> ---
> 
> (Updated Aug. 25, 2015, 11:41 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-647
> https://issues.apache.org/jira/browse/AURORA-647
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Unfortunately the reference forced me to move the location of tables in 
> schema.sql, making the delta look much bigger there.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 
> 7728684560d3111edaa2e29d9738f71bc1d7a9ff 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  02ea355539d6f56c1c861ac83293b649285f43f4 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TaskConfigTypeHandler.java
>  2f58357dd63c46eb69de6a6daedcf027712d7b2e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java
>  9afc3f3fcdcc1b60af2207888993411a55984c8e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbInstanceTaskConfig.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbJobUpdate.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbJobUpdateDetails.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbJobUpdateInstructions.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbStoredJobUpdateDetails.java
>  PRE-CREATION 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  6ffb54f5beef332097fd5c2390b56873a82f0aa9 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> d971aa19982e6095fbf514714df670cabd523d9d 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java
>  3e78c097a7a9252ded8a4a7fc6609ecf5d61c5b5 
> 
> Diff: https://reviews.apache.org/r/37789/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37788: Deb: Clean up changelog generation.

2015-08-26 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 26, 2015, 10:01 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37788/
> ---
> 
> (Updated Aug. 26, 2015, 10:01 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> This sets the maintainer fields, and ensures we mark the changelog entry as 
> released.
> 
> 
> Diffs
> -
> 
>   builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 
> 
> Diff: https://reviews.apache.org/r/37788/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37772: Fix RPM building.

2015-08-25 Thread Kevin Sweeney

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



specs/rpm/Makefile (lines 37 - 38)
<https://reviews.apache.org/r/37772/#comment151814>

This is weird as a user will never be able to upgrade from a release 
version to a nightly.


- Kevin Sweeney


On Aug. 25, 2015, 2:13 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37772/
> ---
> 
> (Updated Aug. 25, 2015, 2:13 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-851
> https://issues.apache.org/jira/browse/AURORA-851
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> This addresses several things needed to get RPM builds working.  The major 
> change is the specification of an RPM Epoch, which will allow the desired 
> version ordering for nightly vs released versions.  If you would like some 
> light reading on this topic, you can find it here: 
> http://www.rpm.org/max-rpm-snapshot/s1-rpm-depend-manual-dependencies.html
> 
> 
> Diffs
> -
> 
>   builder/deb/ubuntu-trusty/build.sh 9c162c728a11a52fb3918dc41f899eb64e3c1084 
>   builder/rpm/centos-7/build.sh 9e8eae94a09d013d01ea405ecb2e554d348e2ce8 
>   specs/rpm/Makefile 77605fe4817ada3cb4601b754a5e4980f8fd1e45 
>   specs/rpm/README.md 2432dc7d1de0ae1becd36a914a446a4211f6c177 
>   specs/rpm/aurora.init.sh  
>   specs/rpm/aurora.logrotate  
>   specs/rpm/aurora.service  
>   specs/rpm/aurora.spec 22c107e4dd012d516a11e25a1d1cb6e702e6a24e 
>   specs/rpm/aurora.startup.sh  
>   specs/rpm/aurora.sysconfig  
>   specs/rpm/clusters.json  
>   specs/rpm/thermos-observer.init.sh  
>   specs/rpm/thermos-observer.logrotate  
>   specs/rpm/thermos-observer.service  
>   specs/rpm/thermos-observer.startup.sh  
>   specs/rpm/thermos-observer.sysconfig  
> 
> Diff: https://reviews.apache.org/r/37772/diff/
> 
> 
> Testing
> ---
> 
> Successfully ran
> ```
> ./build-artifact.sh builder/rpm/centos-7 
> ~/apache-aurora-0.10.0_SNAPSHOT.2015.08.25.tar.gz 0.10.0_SNAPSHOT.2015.08.25
> ```
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37743: Replace realpath to get build working on jenkins.

2015-08-24 Thread Kevin Sweeney

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

Ship it!


on Linux you can use `readlink -f` for this as well

- Kevin Sweeney


On Aug. 24, 2015, 7:27 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37743/
> ---
> 
> (Updated Aug. 24, 2015, 7:27 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> Using this to replace the use of `realpath`, which is non-default and not 
> available on our jenkins machines.
> 
> See relevant discussion here: 
> http://stackoverflow.com/questions/3915040/bash-fish-command-to-print-absolute-path-to-a-file
> 
> 
> Diffs
> -
> 
>   build-artifact.sh 263851c2e485caee2e0733c9cf537d9a951733a8 
> 
> Diff: https://reviews.apache.org/r/37743/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37666: Import of Twitter Commons.

2015-08-24 Thread Kevin Sweeney

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

Ship it!



build.gradle (line 128)
<https://reviews.apache.org/r/37666/#comment151583>

kill newline


- Kevin Sweeney


On Aug. 24, 2015, 4:54 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37666/
> ---
> 
> (Updated Aug. 24, 2015, 4:54 p.m.)
> 
> 
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Maxim Khutornenko, 
> and Bill Farner.
> 
> 
> Bugs: AURORA-1213
> https://issues.apache.org/jira/browse/AURORA-1213
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch imports the Java portion of Twitter Commons into our tree. It 
> copies
> the following files from sha bc7248d:
> 
> src/java/com/twitter/common/application/AbstractApplication.java
> src/java/com/twitter/common/application/AppLauncher.java
> src/java/com/twitter/common/application/Application.java
> src/java/com/twitter/common/application/http/DefaultQuitHandler.java
> src/java/com/twitter/common/application/http/GraphViewer.java
> src/java/com/twitter/common/application/http/HttpAssetConfig.java
> src/java/com/twitter/common/application/http/HttpFilterConfig.java
> src/java/com/twitter/common/application/http/HttpServletConfig.java
> src/java/com/twitter/common/application/http/Registration.java
> src/java/com/twitter/common/application/Lifecycle.java
> src/java/com/twitter/common/application/modules/AppLauncherModule.java
> src/java/com/twitter/common/application/modules/LifecycleModule.java
> src/java/com/twitter/common/application/modules/LocalServiceRegistry.java
> src/java/com/twitter/common/application/modules/LogModule.java
> src/java/com/twitter/common/application/modules/StatsExportModule.java
> src/java/com/twitter/common/application/modules/StatsModule.java
> src/java/com/twitter/common/application/modules/ThriftModule.java
> src/java/com/twitter/common/application/ShutdownRegistry.java
> src/java/com/twitter/common/application/ShutdownStage.java
> src/java/com/twitter/common/application/StartupRegistry.java
> src/java/com/twitter/common/application/StartupStage.java
> src/java/com/twitter/common/args/apt/CmdLineProcessor.java
> src/java/com/twitter/common/args/apt/Configuration.java
> src/java/com/twitter/common/args/Arg.java
> src/java/com/twitter/common/args/ArgFilters.java
> src/java/com/twitter/common/args/ArgParser.java
> src/java/com/twitter/common/args/Args.java
> src/java/com/twitter/common/args/ArgScanner.java
> src/java/com/twitter/common/args/ArgumentInfo.java
> src/java/com/twitter/common/args/CmdLine.java
> src/java/com/twitter/common/args/constraints/CanExecute.java
> src/java/com/twitter/common/args/constraints/CanExecuteFileVerifier.java
> src/java/com/twitter/common/args/constraints/CanRead.java
> src/java/com/twitter/common/args/constraints/CanReadFileVerifier.java
> src/java/com/twitter/common/args/constraints/CanWrite.java
> src/java/com/twitter/common/args/constraints/CanWriteFileVerifier.java
> src/java/com/twitter/common/args/constraints/Exists.java
> src/java/com/twitter/common/args/constraints/ExistsFileVerifier.java
> src/java/com/twitter/common/args/constraints/IsDirectory.java
> src/java/com/twitter/common/args/constraints/IsDirectoryFileVerifier.java
> src/java/com/twitter/common/args/constraints/NotEmpty.java
> src/java/com/twitter/common/args/constraints/NotEmptyIterableVerifier.java
> src/java/com/twitter/common/args/constraints/NotEmptyStringVerifier.java
> src/java/com/twitter/common/args/constraints/NotNegative.java
> src/java/com/twitter/common/args/constraints/NotNegativeNumberVerifier.java
> src/java/com/twitter/common/args/constraints/NotNull.java
> src/java/com/twitter/common/args/constraints/NotNullVerifier.java
> src/java/com/twitter/common/args/constraints/Positive.java
> src/java/com/twitter/common/args/constraints/PositiveNumberVerifier.java
> src/java/com/twitter/common/args/constraints/Range.java
> src/java/com/twitter/common/args/constraints/RangeNumberVerifier.java
> src/java/com/twitter/common/args/NoParser.java
> src/java/com/twitter/common/args/OptionInfo.java
> src/java/com/twitter/common/args/Parser.java
> src/java/com/twitter/common/args/ParserOracle.java
> src/java/com/twitter/common/args/parsers/AmountParser.java
> src/java/com/twitter/common/args/parsers/BooleanParser.java
> src/java/com/twitter/common/args/parsers/ByteParser.java
> src/java/com/twitter

Re: Review Request 37731: Parameterize artifact version in builders.

2015-08-24 Thread Kevin Sweeney


> On Aug. 24, 2015, 3:07 p.m., Kevin Sweeney wrote:
> > build-artifact.sh, line 30
> > <https://reviews.apache.org/r/37731/diff/1/?file=1048757#file1048757line30>
> >
> > For the RPM spec consider making this argument an explicit command-line 
> > flag (with `--define`) rather than an environment variable.
> 
> Bill Farner wrote:
> That's what's done, but the Makefile consumes it as an env var: 
> https://github.com/apache/aurora-packaging/blob/master/specs/rpm/Makefile#L57
> 
> Bill Farner wrote:
> Scratch that, the Makefile reads `.auroraversion`.  Updated patch 
> incoming to allow override via env var to the Makefile.
> 
> Bill Farner wrote:
> Last edit - it uses `?=`, so no change needed.

Not necessary to read the environment variable within the Makefile - you can 
call it with make `AURORA_VERSION=$AURORA_VERSION` ...


- Kevin


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


On Aug. 24, 2015, 3:02 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37731/
> -------
> 
> (Updated Aug. 24, 2015, 3:02 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1410
> https://issues.apache.org/jira/browse/AURORA-1410
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> This allows the builder to set the aurora version number.  The 
> `AURORA_VERSION` environment variable is already read by the RPM spec.
> 
> 
> Diffs
> -
> 
>   README.md e45f22fed3a7a6d04db08481d2d87bf22ebfdd1a 
>   build-artifact.sh e6ea723331db34e8d0e8462c7cdbf66ec4d4814e 
>   builder/deb/ubuntu-trusty/build.sh 6bee0e7bd7fe81682b583041cf9ae32bfaec33f0 
> 
> Diff: https://reviews.apache.org/r/37731/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./build-artifact.sh \
>   builder/deb/ubuntu-trusty \
>   ~/Downloads/apache-aurora-0.9.0.tar.gz \
>   0.9.0
> ```
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37731: Parameterize artifact version in builders.

2015-08-24 Thread Kevin Sweeney

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

Ship it!



build-artifact.sh (line 30)
<https://reviews.apache.org/r/37731/#comment151543>

For the RPM spec consider making this argument an explicit command-line 
flag (with `--define`) rather than an environment variable.


- Kevin Sweeney


On Aug. 24, 2015, 3:02 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37731/
> ---
> 
> (Updated Aug. 24, 2015, 3:02 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1410
> https://issues.apache.org/jira/browse/AURORA-1410
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> This allows the builder to set the aurora version number.  The 
> `AURORA_VERSION` environment variable is already read by the RPM spec.
> 
> 
> Diffs
> -
> 
>   README.md e45f22fed3a7a6d04db08481d2d87bf22ebfdd1a 
>   build-artifact.sh e6ea723331db34e8d0e8462c7cdbf66ec4d4814e 
>   builder/deb/ubuntu-trusty/build.sh 6bee0e7bd7fe81682b583041cf9ae32bfaec33f0 
> 
> Diff: https://reviews.apache.org/r/37731/diff/
> 
> 
> Testing
> ---
> 
> ```
> ./build-artifact.sh \
>   builder/deb/ubuntu-trusty \
>   ~/Downloads/apache-aurora-0.9.0.tar.gz \
>   0.9.0
> ```
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37727: Deb: Remove defunct default argument.

2015-08-24 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 24, 2015, 1:10 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37727/
> ---
> 
> (Updated Aug. 24, 2015, 1:10 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora-packaging
> 
> 
> Description
> ---
> 
> The `-enable_beta_updater` argument no longer exists.
> 
> 
> Diffs
> -
> 
>   specs/debian/aurora-scheduler.default 
> bc306275d2abac9ebfcfef5c3157cf23b40150b8 
> 
> Diff: https://reviews.apache.org/r/37727/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 37666: Import of Twitter Commons.

2015-08-21 Thread Kevin Sweeney

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



build.gradle (line 93)
<https://reviews.apache.org/r/37666/#comment151239>

typo - should be "servlet"



build.gradle (line 123)
<https://reviews.apache.org/r/37666/#comment151243>

An explanation of why this needs to be a separate project (annotation 
processors the rest of the build depends on) would be good form.



commons-args/src/main/java/com/twitter/common/args/Arg.java (line 1)
<https://reviews.apache.org/r/37666/#comment151240>

Strip the Twitter header here and replace it with the standard 
license-header.



commons-args/src/main/java/com/twitter/common/args/ArgParser.java (line 1)
<https://reviews.apache.org/r/37666/#comment151241>

Missing license header. Consider tooling to automated adding the standard 
apache one and removing the twitter one.



commons-args/src/main/java/com/twitter/common/args/VerifierFor.java (line 1)
<https://reviews.apache.org/r/37666/#comment151242>

Missing license header


- Kevin Sweeney


On Aug. 20, 2015, 6:04 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37666/
> ---
> 
> (Updated Aug. 20, 2015, 6:04 p.m.)
> 
> 
> Review request for Aurora, Jake Farrell, Kevin Sweeney, Maxim Khutornenko, 
> and Bill Farner.
> 
> 
> Bugs: AURORA-1213
> https://issues.apache.org/jira/browse/AURORA-1213
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch imports the Java portion of Twitter Commons into our tree. It 
> copies
> the following files from sha bc7248d:
> 
> src/java/com/twitter/common/application/AbstractApplication.java
> src/java/com/twitter/common/application/AppLauncher.java
> src/java/com/twitter/common/application/Application.java
> src/java/com/twitter/common/application/http/DefaultQuitHandler.java
> src/java/com/twitter/common/application/http/GraphViewer.java
> src/java/com/twitter/common/application/http/HttpAssetConfig.java
> src/java/com/twitter/common/application/http/HttpFilterConfig.java
> src/java/com/twitter/common/application/http/HttpServletConfig.java
> src/java/com/twitter/common/application/http/Registration.java
> src/java/com/twitter/common/application/Lifecycle.java
> src/java/com/twitter/common/application/modules/AppLauncherModule.java
> src/java/com/twitter/common/application/modules/LifecycleModule.java
> src/java/com/twitter/common/application/modules/LocalServiceRegistry.java
> src/java/com/twitter/common/application/modules/LogModule.java
> src/java/com/twitter/common/application/modules/StatsExportModule.java
> src/java/com/twitter/common/application/modules/StatsModule.java
> src/java/com/twitter/common/application/modules/ThriftModule.java
> src/java/com/twitter/common/application/ShutdownRegistry.java
> src/java/com/twitter/common/application/ShutdownStage.java
> src/java/com/twitter/common/application/StartupRegistry.java
> src/java/com/twitter/common/application/StartupStage.java
> src/java/com/twitter/common/args/apt/CmdLineProcessor.java
> src/java/com/twitter/common/args/apt/Configuration.java
> src/java/com/twitter/common/args/Arg.java
> src/java/com/twitter/common/args/ArgFilters.java
> src/java/com/twitter/common/args/ArgParser.java
> src/java/com/twitter/common/args/Args.java
> src/java/com/twitter/common/args/ArgScanner.java
> src/java/com/twitter/common/args/ArgumentInfo.java
> src/java/com/twitter/common/args/CmdLine.java
> src/java/com/twitter/common/args/constraints/CanExecute.java
> src/java/com/twitter/common/args/constraints/CanExecuteFileVerifier.java
> src/java/com/twitter/common/args/constraints/CanRead.java
> src/java/com/twitter/common/args/constraints/CanReadFileVerifier.java
> src/java/com/twitter/common/args/constraints/CanWrite.java
> src/java/com/twitter/common/args/constraints/CanWriteFileVerifier.java
> src/java/com/twitter/common/args/constraints/Exists.java
> src/java/com/twitter/common/args/constraints/ExistsFileVerifier.java
> src/java/com/twitter/common/args/constraints/IsDirectory.java
> src/java/com/twitter/common/args/constraints/IsDirectoryFileVerifier.java
> src/java/com/twitter/common/args/constraints/NotEmpty.java
> src/java/com/twitter/common/args/constraints/NotEmptyIterableVerifier.java
> src/java/com/twitter/common/args/constraints/NotEmptyStringVerifier.java
> src/java/com/twitter/common/args/constraints/NotNegative.java
> src/java/com/twitter/common/args/constraint

Re: Review Request 36289: Custom executor support for Scheduler

2015-08-19 Thread Kevin Sweeney
.java (line 170)
<https://reviews.apache.org/r/36289/#comment151050>

This needs to re-throw, otherwise initialization will continue and users 
will experience weird runtime behavior. Consider removing the custom exception 
type and throwing `IllegalArgumentException` in the parser.



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
(lines 211 - 213)
<https://reviews.apache.org/r/36289/#comment151051>

Formatting is weird - push the call chain to the next line:

```java
ResourceSlot.from(request.getTask())

.withOverhead(requireNonNull(executorSettings.get(task.getExecutorConfig().getName());
```

This is a weird (though correct) place for a `requireNonNull` check though 
because the scheduler can crash in its scheduling loop in the case of a config 
change.

I think you need to add logic to make sure that every task has an executor 
that's acceptible to the system at startup.


- Kevin Sweeney


On Aug. 17, 2015, 11:10 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated Aug. 17, 2015, 11:10 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for dynamically chosing an executor that's definied in a server 
> side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single 
> ExecutorSettings object.
> 
> Future:
> ===
> Create an offshoot of the current client that allows to send thrift calls 
> with different executor configs which will allow use of custom executors. 
> Some work on this has already been done and will be published ASAP for 
> testing.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 
> 8162323816aedc711a3af84cd499250b78718ab3 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 50e7fc91108993e547869df5b9e5c925fb89a225 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 37772d0b75d022f072af10d82d096981680e193f 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  608af1afe6fc27c8c597490e88fed75580076c95 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  b2327a47374d81b59886c1e4575ded8340322db7 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> 6a80503aeb2058e8f8065285adc151197d2d14d6 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java 
> a1ac922d471013779710e02c0c9ca9f84b506807 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java

Re: Review Request 37379: Updating to Mesos 0.23.0.

2015-08-17 Thread Kevin Sweeney

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



build-support/python/make-mesos-native-egg (line 122)
<https://reviews.apache.org/r/37379/#comment150708>

Have you tested this? You use the same heredoc delimiter "EOS" twice.


- Kevin Sweeney


On Aug. 17, 2015, 11:37 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37379/
> ---
> 
> (Updated Aug. 17, 2015, 11:37 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1429
> https://issues.apache.org/jira/browse/AURORA-1429
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updating to Mesos 0.23.0. A few notes:
> - Thermos executor fails to initialize MesosExecutorDriver unless 
> libcurl4-nss-dev is installed.
> - Building Mesos now requires gcc 4.8, which is not available on centos6 via 
> a standard repo.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 4bc82b8b55ec14c8215e0bd67c4e68200ad9dbfc 
>   NEWS 6c0e20fe93e765dc84c3ba860cb8af5d565ba1ca 
>   build-support/python/make-mesos-native-egg 
> fce53e42dde72b893f291db9d78af1dc47fafe2f 
>   build.gradle afb9b808a215eb5ff6e34ec6908c0e1ffbcd1991 
>   docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 
>   examples/vagrant/provision-dev-cluster.sh 
> fc5388507deae3ea6f14eca3cd4cffd598e2422f 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java 
> 23c0e53b1a708843a9722c8bf1e8f0b739a9f5eb 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> aeb8946f8ebc58e6608557a0f277e382dc2667e5 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile 
> e7f7f541e340c426b4e2a0f7a2c326af4bf902c2 
> 
> Diff: https://reviews.apache.org/r/37379/diff/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37379: Updating to Mesos 0.23.0.

2015-08-17 Thread Kevin Sweeney

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

Ship it!


LGTM once remaining review comments are addressed.

- Kevin Sweeney


On Aug. 17, 2015, 11:37 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37379/
> ---
> 
> (Updated Aug. 17, 2015, 11:37 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1429
> https://issues.apache.org/jira/browse/AURORA-1429
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updating to Mesos 0.23.0. A few notes:
> - Thermos executor fails to initialize MesosExecutorDriver unless 
> libcurl4-nss-dev is installed.
> - Building Mesos now requires gcc 4.8, which is not available on centos6 via 
> a standard repo.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 4bc82b8b55ec14c8215e0bd67c4e68200ad9dbfc 
>   NEWS 6c0e20fe93e765dc84c3ba860cb8af5d565ba1ca 
>   build-support/python/make-mesos-native-egg 
> fce53e42dde72b893f291db9d78af1dc47fafe2f 
>   build.gradle afb9b808a215eb5ff6e34ec6908c0e1ffbcd1991 
>   docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 
>   examples/vagrant/provision-dev-cluster.sh 
> fc5388507deae3ea6f14eca3cd4cffd598e2422f 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java 
> 23c0e53b1a708843a9722c8bf1e8f0b739a9f5eb 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> aeb8946f8ebc58e6608557a0f277e382dc2667e5 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile 
> e7f7f541e340c426b4e2a0f7a2c326af4bf902c2 
> 
> Diff: https://reviews.apache.org/r/37379/diff/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37379: Updating to Mesos 0.23.0.

2015-08-17 Thread Kevin Sweeney

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



NEWS (line 12)
<https://reviews.apache.org/r/37379/#comment150691>

It won't necessarily be libcurl-nss (as the suffix will be dependent on 
which SSL provider libcurl was compiled with), I think it's sufficient to 
reference libcurl here.

Rather than mention MesosExecutorDriver (an implementation detail) just 
mention the executor.

Rather than reference a blob-link just reference docs/getting-started.md - 
the NEWS file is in the same repo.


- Kevin Sweeney


On Aug. 17, 2015, 11:37 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37379/
> ---
> 
> (Updated Aug. 17, 2015, 11:37 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1429
> https://issues.apache.org/jira/browse/AURORA-1429
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updating to Mesos 0.23.0. A few notes:
> - Thermos executor fails to initialize MesosExecutorDriver unless 
> libcurl4-nss-dev is installed.
> - Building Mesos now requires gcc 4.8, which is not available on centos6 via 
> a standard repo.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 4bc82b8b55ec14c8215e0bd67c4e68200ad9dbfc 
>   NEWS 6c0e20fe93e765dc84c3ba860cb8af5d565ba1ca 
>   build-support/python/make-mesos-native-egg 
> fce53e42dde72b893f291db9d78af1dc47fafe2f 
>   build.gradle afb9b808a215eb5ff6e34ec6908c0e1ffbcd1991 
>   docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 
>   examples/vagrant/provision-dev-cluster.sh 
> fc5388507deae3ea6f14eca3cd4cffd598e2422f 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java 
> 23c0e53b1a708843a9722c8bf1e8f0b739a9f5eb 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> aeb8946f8ebc58e6608557a0f277e382dc2667e5 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile 
> e7f7f541e340c426b4e2a0f7a2c326af4bf902c2 
> 
> Diff: https://reviews.apache.org/r/37379/diff/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37379: Updating to Mesos 0.23.0.

2015-08-17 Thread Kevin Sweeney


> On Aug. 13, 2015, 4:53 p.m., Kevin Sweeney wrote:
> > src/test/sh/org/apache/aurora/e2e/Dockerfile, line 18
> > <https://reviews.apache.org/r/37379/diff/1/?file=1038072#file1038072line18>
> >
> > Is this documented upstream or just because we use libcurl4-nss-dev in 
> > the egg builder script? Either way you can install just libcurl4-nss rather 
> > than the -dev version of the package.
> 
> Maxim Khutornenko wrote:
> It's documented indirectly here: 
> https://github.com/apache/mesos/blob/cd4173059489eae33ee0c578e1fd61f10fc45a66/support/jenkins_build.sh#L57

Were you successful at using libcurl4-nss without the -dev package? I imagine 
the difference is quite a bloated Docker image and would like this repo to 
reflect the minimum for the test to pass.


- Kevin


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


On Aug. 17, 2015, 11:37 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37379/
> ---
> 
> (Updated Aug. 17, 2015, 11:37 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1429
> https://issues.apache.org/jira/browse/AURORA-1429
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updating to Mesos 0.23.0. A few notes:
> - Thermos executor fails to initialize MesosExecutorDriver unless 
> libcurl4-nss-dev is installed.
> - Building Mesos now requires gcc 4.8, which is not available on centos6 via 
> a standard repo.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 4bc82b8b55ec14c8215e0bd67c4e68200ad9dbfc 
>   NEWS 6c0e20fe93e765dc84c3ba860cb8af5d565ba1ca 
>   build-support/python/make-mesos-native-egg 
> fce53e42dde72b893f291db9d78af1dc47fafe2f 
>   build.gradle afb9b808a215eb5ff6e34ec6908c0e1ffbcd1991 
>   docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 
>   examples/vagrant/provision-dev-cluster.sh 
> fc5388507deae3ea6f14eca3cd4cffd598e2422f 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java 
> 23c0e53b1a708843a9722c8bf1e8f0b739a9f5eb 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> aeb8946f8ebc58e6608557a0f277e382dc2667e5 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile 
> e7f7f541e340c426b4e2a0f7a2c326af4bf902c2 
> 
> Diff: https://reviews.apache.org/r/37379/diff/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37379: Updating to Mesos 0.23.0.

2015-08-17 Thread Kevin Sweeney


> On Aug. 13, 2015, 4:53 p.m., Kevin Sweeney wrote:
> > build-support/python/make-mesos-native-egg, line 119
> > <https://reviews.apache.org/r/37379/diff/1/?file=1038066#file1038066line119>
> >
> > HTTPS?
> 
> Maxim Khutornenko wrote:
> No htts endpoint exists. This hangs forever:
> ```
> Resolving people.centos.org... 204.15.73.242
> Connecting to people.centos.org|204.15.73.242|:443...
> ```

Hmm - upstream packages are also completely unsigned 
(http://people.centos.org/tru/devtools-2/readme) - perhaps drop a TODO to use 
signed packages when they become available.


- Kevin


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


On Aug. 17, 2015, 11:37 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37379/
> ---
> 
> (Updated Aug. 17, 2015, 11:37 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1429
> https://issues.apache.org/jira/browse/AURORA-1429
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updating to Mesos 0.23.0. A few notes:
> - Thermos executor fails to initialize MesosExecutorDriver unless 
> libcurl4-nss-dev is installed.
> - Building Mesos now requires gcc 4.8, which is not available on centos6 via 
> a standard repo.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 4bc82b8b55ec14c8215e0bd67c4e68200ad9dbfc 
>   NEWS 6c0e20fe93e765dc84c3ba860cb8af5d565ba1ca 
>   build-support/python/make-mesos-native-egg 
> fce53e42dde72b893f291db9d78af1dc47fafe2f 
>   build.gradle afb9b808a215eb5ff6e34ec6908c0e1ffbcd1991 
>   docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 
>   examples/vagrant/provision-dev-cluster.sh 
> fc5388507deae3ea6f14eca3cd4cffd598e2422f 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java 
> 23c0e53b1a708843a9722c8bf1e8f0b739a9f5eb 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> aeb8946f8ebc58e6608557a0f277e382dc2667e5 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile 
> e7f7f541e340c426b4e2a0f7a2c326af4bf902c2 
> 
> Diff: https://reviews.apache.org/r/37379/diff/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37379: Updating to Mesos 0.23.0.

2015-08-13 Thread Kevin Sweeney

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



NEWS (line 11)
<https://reviews.apache.org/r/37379/#comment150266>

Replace with "Upgraded mesos to 0.23.0"



build-support/python/make-mesos-native-egg (line 119)
<https://reviews.apache.org/r/37379/#comment150267>

HTTPS?



src/test/sh/org/apache/aurora/e2e/Dockerfile (line 18)
<https://reviews.apache.org/r/37379/#comment150268>

Is this documented upstream or just because we use libcurl4-nss-dev in the 
egg builder script? Either way you can install just libcurl4-nss rather than 
the -dev version of the package.


- Kevin Sweeney


On Aug. 11, 2015, 9:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37379/
> ---
> 
> (Updated Aug. 11, 2015, 9:58 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1429
> https://issues.apache.org/jira/browse/AURORA-1429
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Updating to Mesos 0.23.0. A few notes:
> - Thermos executor fails to initialize MesosExecutorDriver unless 
> libcurl4-nss-dev is installed.
> - Building Mesos now requires gcc 4.8, which is not available on centos6 via 
> a standard repo.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 4bc82b8b55ec14c8215e0bd67c4e68200ad9dbfc 
>   NEWS 6c0e20fe93e765dc84c3ba860cb8af5d565ba1ca 
>   build-support/python/make-mesos-native-egg 
> fce53e42dde72b893f291db9d78af1dc47fafe2f 
>   build.gradle afb9b808a215eb5ff6e34ec6908c0e1ffbcd1991 
>   docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 
>   examples/vagrant/provision-dev-cluster.sh 
> fc5388507deae3ea6f14eca3cd4cffd598e2422f 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java 
> 23c0e53b1a708843a9722c8bf1e8f0b739a9f5eb 
>   src/test/java/org/apache/aurora/scheduler/app/local/FakeMaster.java 
> aeb8946f8ebc58e6608557a0f277e382dc2667e5 
>   src/test/sh/org/apache/aurora/e2e/Dockerfile 
> e7f7f541e340c426b4e2a0f7a2c326af4bf902c2 
> 
> Diff: https://reviews.apache.org/r/37379/diff/
> 
> 
> Testing
> ---
> 
> ./src/test/sh/org/apache/aurora/e2e/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 37447: Making scheduler loop continue after mismatch.

2015-08-13 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On Aug. 13, 2015, 2:25 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37447/
> ---
> 
> (Updated Aug. 13, 2015, 2:25 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Zameer Manji.
> 
> 
> Bugs: AURORA-1435
> https://issues.apache.org/jira/browse/AURORA-1435
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Dropping the return statement that prematurely terminated scheduling loop on 
> a first mismatch.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 14d70df1f86e87ccc299558326b69662af759d08 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 1de1d1f11a57ac6f31f184d57776bc0d0256a901 
> 
> Diff: https://reviews.apache.org/r/37447/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Review Request 37047: Build Kerberos clients in RPM.

2015-08-03 Thread Kevin Sweeney

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

Review request for Aurora, Steve Salevan and Bill Farner.


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


Repository: aurora


Description
---

Build Kerberos clients in RPM.


Diffs
-

  build-support/packaging/rpm/aurora.spec 
3003ced1451f20b8e33d8532645fab85fffbca74 

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


Testing
---

None - I don't have a test environment for RPMs.


Thanks,

Kevin Sweeney



Re: Review Request 36874: Fix typo in the scheduler deployment documentation

2015-08-03 Thread Kevin Sweeney

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


ping

- Kevin Sweeney


On July 27, 2015, 8:01 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36874/
> ---
> 
> (Updated July 27, 2015, 8:01 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix typo in scheduler deployment documentation
> 
> The command should be run on schedulers, not masters.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 
> 
> Diff: https://reviews.apache.org/r/36874/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 36972: One python_library per exported setup.py project

2015-08-03 Thread Kevin Sweeney


> On July 31, 2015, 3:21 p.m., Brian Wickman wrote:
> > I think it makes sense to split into two binary-exporting packages: one 
> > client-side and one server-side.  The client side (aurora.client) should 
> > contain aurora client and aurora admin client. The server-side (reuse 
> > aurora.executor?) should contain aurora executor, thermos cli, thermos 
> > observer and thermos runner.
> 
> Kevin Sweeney wrote:
> I like this suggestion and would like to refactor the code to fit it into 
> this framework. Would you accept that in a follow-up review?
> 
> Specifically there would then be 3 top-level packages (5 if you count the 
> thrift API bindings, which I would also like to export to allow folks to 
> write clients)
> 
> ```
> src/main/python/apache/aurora
>   common/
>   client/
>   executor/
> ```
> 
> ```
> % pip install apache.aurora.executor
> # thermos
> # thermos-executor
> # thermos-observer
> # thermos-runner
> 
> % pip install apache.aurora.client
> # aurora
> # aurora-admin
> ```
> 
> Everything else would remain more-or-less organized the same but move to 
> a subpackage of one of the top-level targets. The rule is simple - if both 
> server-side and client-side import something it goes under common/, otherwise 
> it goes under client/ or executor/ as appropriate.
> 
> Brian Wickman wrote:
> This seems reasonable.  I'd prefer to keep the name aurora-executor but 
> it probably merits public debate.

The name aurora-executor is fine with me, in this case I've left it unchanged 
as "thermos_executor". But I agree a debate on the proposed renaming can take 
place on another review.


- Kevin


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


On Aug. 1, 2015, 3:38 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36972/
> ---
> 
> (Updated Aug. 1, 2015, 3:38 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a mostly red code diff and makes the pants build work similar to how 
> it already works in an IDE environment while minimizing duplicated 
> information. This change also removes the confusing versions of `thermos` and 
> `thermos_observer` in favor of new ones. Because of the way "covering 
> dependencies" were required it was easier to do this refactor than more 
> tactically create a working `setup_py` target for `apache.aurora.tools` 
> without cycles and without duplicated files.
> 
> * Remove the `apache.thermos` package.
> * Rename the `apache.gen.aurora` package to `apache.aurora.thrift`.
> * Rename the `apache.gen.thermos` package to `apache.thermos.thrift`.
> * Introduce a new `apache.aurora.tools` package with the `thermos` and 
> `thermos_observer` binaries.
> * Create apache.thermos.runner package, rename `thermos/bin` to 
> `thermos/runner`.
> * Remove all `*-packaged` and virtual dependency trees, as well as the phrase 
> "covering dependencies."
> * Use `_`-prefix naming convention for private target names.
> * Replace manual list of targets in `make-python-sdists` with 
> automatically-generated one (using new convention).
> * Introduce a new `apache.aurora.kerberos` package with the `kaurora` and 
> `kaurora_admin` binaries.
> * Remove all `BUILD` files in `src/main` that don't contain an exported 
> `setup.py` library (except one mentioned in TODO).
> * Use dictionary syntax in `with_binaries`.
> * Remove unused/unreachable binaries (most `bin/` dirs under `thermos/`).
> * Create blank `__init__.py` files in directories that pants warns aren't 
> packages.
> * Added `thermos` command to vagrant image.
> 
> 
> Diffs
> -
> 
>   NEWS 0a8500c524a92eaf51384125687128d2fbbb0b53 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> d196fefc2c5e5ee32d0cf9c901cffe7d247379d1 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> d0d789a6ee3971e3070f9397d53929563a77f7ea 
>   build-support/packaging/debian/rules 
> 17e00c02a3cb3294d5107516d795a73587ca4f70 
>   build-support/packaging/rpm/aurora.spec 
> 7cf8de6f7b99788ca461a90fc4aefba7dccd7b63 
>   build-support/

Re: Review Request 36972: One python_library per exported setup.py project

2015-08-01 Thread Kevin Sweeney
che/thermos/cli/bin/thermos.py 
fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
  src/main/python/apache/thermos/cli/commands/BUILD 
552eeb4f61693033d8828e789bd22e680b957d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing (updated)
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
./build-support/python/make-pycharm-virtualenv

Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.

Documentation pushed to 
https://github.com/kevints/aurora/blob/36972/docs/build-system.md


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney
che/thermos/cli/commands/BUILD 
552eeb4f61693033d8828e789bd22e680b957d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.

Documentation pushed to 
https://github.com/kevints/aurora/blob/36972/docs/build-system.md


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney


> On July 31, 2015, 3:57 p.m., Joshua Cohen wrote:
> > I'm -1 to shipping without something under docs/ explaining this 
> > organizational structure.
> 
> Bill Farner wrote:
> maybe a README.md under `src/main/python/apache` instead?

I've added a new file docs/build-system.md


- Kevin


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


On July 31, 2015, 4:18 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36972/
> ---
> 
> (Updated July 31, 2015, 4:18 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a mostly red code diff and makes the pants build work similar to how 
> it already works in an IDE environment while minimizing duplicated 
> information. This change also removes the confusing versions of `thermos` and 
> `thermos_observer` in favor of new ones. Because of the way "covering 
> dependencies" were required it was easier to do this refactor than more 
> tactically create a working `setup_py` target for `apache.aurora.tools` 
> without cycles and without duplicated files.
> 
> * Remove the `apache.thermos` package.
> * Rename the `apache.gen.aurora` package to `apache.aurora.thrift`.
> * Rename the `apache.gen.thermos` package to `apache.thermos.thrift`.
> * Introduce a new `apache.aurora.tools` package with the `thermos` and 
> `thermos_observer` binaries.
> * Create apache.thermos.runner package, rename `thermos/bin` to 
> `thermos/runner`.
> * Remove all `*-packaged` and virtual dependency trees, as well as the phrase 
> "covering dependencies."
> * Use `_`-prefix naming convention for private target names.
> * Replace manual list of targets in `make-python-sdists` with 
> automatically-generated one (using new convention).
> * Introduce a new `apache.aurora.kerberos` package with the `kaurora` and 
> `kaurora_admin` binaries.
> * Remove all `BUILD` files in `src/main` that don't contain an exported 
> `setup.py` library (except one mentioned in TODO).
> * Use dictionary syntax in `with_binaries`.
> * Remove unused/unreachable binaries (most `bin/` dirs under `thermos/`).
> * Create blank `__init__.py` files in directories that pants warns aren't 
> packages.
> * Added `thermos` command to vagrant image.
> 
> 
> Diffs
> -
> 
>   NEWS 0a8500c524a92eaf51384125687128d2fbbb0b53 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> d196fefc2c5e5ee32d0cf9c901cffe7d247379d1 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> d0d789a6ee3971e3070f9397d53929563a77f7ea 
>   build-support/packaging/debian/rules 
> 17e00c02a3cb3294d5107516d795a73587ca4f70 
>   build-support/packaging/rpm/aurora.spec 
> 7cf8de6f7b99788ca461a90fc4aefba7dccd7b63 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
>   docs/README.md 9893763cb75faf1aeb66ed905e9f696d4b532d16 
>   docs/build-system.md PRE-CREATION 
>   examples/vagrant/aurorabuild.sh fbaa6ae9ef7ff2910af8c9c0d6b8ef90ea3e152a 
>   src/main/python/apache/aurora/admin/BUILD 
> 22bf3f9943ea11258ba681bdb80feb00206bb926 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> 1c39717b656465bb1966073340f9fe80be01a085 
>   src/main/python/apache/aurora/client/BUILD 
> e73cd52289209bb9658b16bb77dc0b0a9c811a1a 
>   src/main/python/apache/aurora/client/api/BUILD 
> a030a67b78fc4bc4682d0df169e27efc6810dce3 
>   src/main/python/apache/aurora/client/cli/BUILD 
> 973d05971bd940c7e38c48ce6bfbf5c8e1654c5a 
>   src/main/python/apache/aurora/client/cli/client.py 
> c0974f3bef59f8f7c2320398d367cb4dd9048d2b 
>   src/main/python/apache/aurora/client/hooks/BUILD 
> ddf813b5d2e7d63507a8e08745ebf6cb3dbac8e1 
>   src/main/python/apache/aurora/common/BUILD 
> abc122b0775bb17b1df67bdb946c472010219b9b 
>   src/main/python/apache/aurora/common/auth/BUILD 
> 0abac94eeeb71e6af43ed191ea690e5f96a6be23 
>   src/main/python/apache/aurora/common/auth/kerberos.py 
> 2d782b63b611a9d2604ef0ab1116d3e68fb86dc7 
>   src/main/python/apache/aurora/config/BUILD 
> 0a3a93fa6bb785903f71bc067aecc79c0e45a0b5 
>   src/main/python/apache/aurora/config/schema/BUILD 
> 171f42a24ec7ddc2846eb68b6a60e7d8dec4383b 
>   src/main/python/apache/aurora/executor/BUILD 
&g

Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney
che/thermos/cli/bin/thermos.py 
fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
  src/main/python/apache/thermos/cli/commands/BUILD 
552eeb4f61693033d8828e789bd22e680b957d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing (updated)
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.

Documentation pushed to 
https://github.com/kevints/aurora/blob/36972/docs/build-system.md


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney
7d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney


> On July 31, 2015, 3:21 p.m., Brian Wickman wrote:
> > I think it makes sense to split into two binary-exporting packages: one 
> > client-side and one server-side.  The client side (aurora.client) should 
> > contain aurora client and aurora admin client. The server-side (reuse 
> > aurora.executor?) should contain aurora executor, thermos cli, thermos 
> > observer and thermos runner.

I like this suggestion and would like to refactor the code to fit it into this 
framework. Would you accept that in a follow-up review?

Specifically there would then be 3 top-level packages (5 if you count the 
thrift API bindings, which I would also like to export to allow folks to write 
clients)

```
src/main/python/apache/aurora
  common/
  client/
  executor/
```

```
% pip install apache.aurora.executor
# thermos
# thermos-executor
# thermos-observer
# thermos-runner

% pip install apache.aurora.client
# aurora
# aurora-admin
```

Everything else would remain more-or-less organized the same but move to a 
subpackage of one of the top-level targets. The rule is simple - if both 
server-side and client-side import something it goes under common/, otherwise 
it goes under client/ or executor/ as appropriate.


- Kevin


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


On July 31, 2015, 3:30 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36972/
> ---
> 
> (Updated July 31, 2015, 3:30 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Publishing this review now for early feedback, don't intend to ship until 
> most of the TODOs are addressed.
> 
> This is a mostly red code diff and makes the pants build work similar to how 
> it already works in an IDE environment while minimizing duplicated 
> information. This change also removes the confusing versions of `thermos` and 
> `thermos_observer` in favor of new ones. Because of the way "covering 
> dependencies" were required it was easier to do this refactor than more 
> tactically create a working `setup_py` target for `apache.aurora.tools` 
> without cycles and without duplicated files.
> 
> * Remove the `apache.thermos` package.
> * Rename the `apache.gen.aurora` package to `apache.aurora.thrift`.
> * Rename the `apache.gen.thermos` package to `apache.thermos.thrift`.
> * Introduce a new `apache.aurora.tools` package with the `thermos` and 
> `thermos_observer` binaries.
> * Create apache.thermos.runner package, rename `thermos/bin` to 
> `thermos/runner`.
> * Remove all `*-packaged` and virtual dependency trees, as well as the phrase 
> "covering dependencies."
> * Use `_`-prefix naming convention for private target names.
> * Replace manual list of targets in `make-python-sdists` with 
> automatically-generated one (using new convention).
> * Introduce a new `apache.aurora.kerberos` package with the `kaurora` and 
> `kaurora_admin` binaries.
> * Remove all `BUILD` files in `src/main` that don't contain an exported 
> `setup.py` library (except one mentioned in TODO).
> * Use dictionary syntax in `with_binaries`.
> * Remove unused/unreachable binaries (most `bin/` dirs under `thermos/`).
> * Create blank `__init__.py` files in directories that pants warns aren't 
> packages.
> * Added `thermos` command to vagrant image.
> 
> 
> Diffs
> -
> 
>   NEWS 0a8500c524a92eaf51384125687128d2fbbb0b53 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> d196fefc2c5e5ee32d0cf9c901cffe7d247379d1 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> d0d789a6ee3971e3070f9397d53929563a77f7ea 
>   build-support/packaging/debian/rules 
> 17e00c02a3cb3294d5107516d795a73587ca4f70 
>   build-support/packaging/rpm/aurora.spec 
> 7cf8de6f7b99788ca461a90fc4aefba7dccd7b63 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
>   examples/vagrant/aurorabuild.sh fbaa6ae9ef7ff2910af8c9c0d6b8ef90ea3e152a 
>   src/main/python/apache/aurora/admin/BUILD 
> 22bf3f9943ea11258ba681bdb80feb00206bb926 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> 1c39717b656465bb1966073340f9fe80be01a085 
>   src/main/python/apache/aurora/client/BUILD 
> e73cd52289209bb9658b16bb77dc0b0a9c811a1a 
>   src/main/python/apache/aurora/client/a

Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney
n/apache/thermos/cli/commands/BUILD 
552eeb4f61693033d8828e789bd22e680b957d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney
n/apache/thermos/cli/bin/thermos.py 
fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
  src/main/python/apache/thermos/cli/commands/BUILD 
552eeb4f61693033d8828e789bd22e680b957d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing (updated)
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh

Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.


Thanks,

Kevin Sweeney



Review Request 36998: Use hostname instead of IP in Kerberos end to end test.

2015-07-31 Thread Kevin Sweeney

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

Review request for Aurora and Bill Farner.


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


Repository: aurora


Description
---

Use hostname instead of IP in Kerberos end to end test.


Diffs
-

  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
02c9e6a4109376c62b93d75921686548d2d795f9 
  src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 
760997c8420e9e430912de406cef494a9118897a 

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


Testing
---

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


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney


> On July 31, 2015, 9:01 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/admin/BUILD, lines 84-92
> > <https://reviews.apache.org/r/36972/diff/2/?file=1025863#file1025863line84>
> >
> > Are we completely abondoning the idea of having a non-kerberized client 
> > version with this? This was one of the sticking points in 
> > https://reviews.apache.org/r/32541/.
> 
> Bill Farner wrote:
> FWIW i think we should indeed abandon that idea.  The goal of 
> reducing/eliminating native dependncies is a good one, but i don't find it 
> beneficial to distribute different builds based on the features present.

Given where this was factored it would've added a new native dependency to the 
executor which could complicate our already not-great deployment story. I've 
kicked it into its own package for now but happy to revisit in another diff.


- Kevin


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


On July 31, 2015, 11:03 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36972/
> ---
> 
> (Updated July 31, 2015, 11:03 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Publishing this review now for early feedback, don't intend to ship until 
> most of the TODOs are addressed.
> 
> This is a mostly red code diff and makes the pants build work similar to how 
> it already works in an IDE environment while minimizing duplicated 
> information. This change also removes the confusing versions of `thermos` and 
> `thermos_observer` in favor of new ones. Because of the way "covering 
> dependencies" were required it was easier to do this refactor than more 
> tactically create a working `setup_py` target for `apache.aurora.tools` 
> without cycles and without duplicated files.
> 
> * Remove the `apache.thermos` package.
> * Rename the `apache.gen.aurora` package to `apache.aurora.thrift`.
> * Rename the `apache.gen.thermos` package to `apache.thermos.thrift`.
> * Introduce a new `apache.aurora.tools` package with the `thermos` and 
> `thermos_observer` binaries.
> * Create apache.thermos.runner package, rename `thermos/bin` to 
> `thermos/runner`.
> * Remove all `*-packaged` and virtual dependency trees, as well as the phrase 
> "covering dependencies."
> * Use `_`-prefix naming convention for private target names.
> * Replace manual list of targets in `make-python-sdists` with 
> automatically-generated one (using new convention).
> * Introduce a new `apache.aurora.kerberos` package with the `kaurora` and 
> `kaurora_admin` binaries.
> * Remove all `BUILD` files in `src/main` that don't contain an exported 
> `setup.py` library (except one mentioned in TODO).
> * Use dictionary syntax in `with_binaries` (TODO: replace the underscores in 
> binary names with dashes?).
> * Remove unused/unreachable binaries (most `bin/` dirs under `thermos/`).
> * Create blank `__init__.py` files in directories that pants warns aren't 
> packages.
> * Added `thermos` command to vagrant image.
> 
> * TODO: Write NEWS entry
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> d196fefc2c5e5ee32d0cf9c901cffe7d247379d1 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> d0d789a6ee3971e3070f9397d53929563a77f7ea 
>   build-support/packaging/debian/rules 
> 17e00c02a3cb3294d5107516d795a73587ca4f70 
>   build-support/packaging/rpm/aurora.spec 
> 7cf8de6f7b99788ca461a90fc4aefba7dccd7b63 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
>   examples/vagrant/aurorabuild.sh fbaa6ae9ef7ff2910af8c9c0d6b8ef90ea3e152a 
>   src/main/python/apache/aurora/admin/BUILD 
> 22bf3f9943ea11258ba681bdb80feb00206bb926 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> 1c39717b656465bb1966073340f9fe80be01a085 
>   src/main/python/apache/aurora/client/BUILD 
> e73cd52289209bb9658b16bb77dc0b0a9c811a1a 
>   src/main/python/apache/aurora/client/api/BUILD 
> a030a67b78fc4bc4682d0df169e27efc6810dce3 
>   src/main/python/apache/aurora/client/cli/BUILD 
> 973d05971bd940c7e38c48ce6bfbf5c8e1654c5a 
>   src/main/python/apache/aurora/client/cli/client.py 
> c0974f3bef59f8f7c2320

Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney
8d33e7ca2005f35c3bdb90ffee6aeade3af 
  src/main/python/apache/thermos/cli/commands/BUILD 
552eeb4f61693033d8828e789bd22e680b957d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists

E2E test gets as far as it usually does.
Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney


> On July 31, 2015, 7:39 a.m., Joshua Cohen wrote:
> > src/main/python/apache/thermos/bin/BUILD, line 40
> > <https://reviews.apache.org/r/36972/diff/2/?file=1025877#file1025877line40>
> >
> > Should this read from .auroraversion?

Good catch! (leftover from my example email). Fixed.


- Kevin


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


On July 31, 2015, 10:56 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36972/
> ---
> 
> (Updated July 31, 2015, 10:56 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Publishing this review now for early feedback, don't intend to ship until 
> most of the TODOs are addressed.
> 
> This is a mostly red code diff and makes the pants build work similar to how 
> it already works in an IDE environment while minimizing duplicated 
> information. This change also removes the confusing versions of `thermos` and 
> `thermos_observer` in favor of new ones. Because of the way "covering 
> dependencies" were required it was easier to do this refactor than more 
> tactically create a working `setup_py` target for `apache.aurora.tools` 
> without cycles and without duplicated files.
> 
> * Remove the `apache.thermos` package.
> * Rename the `apache.gen.aurora` package to `apache.aurora.thrift`.
> * Rename the `apache.gen.thermos` package to `apache.thermos.thrift`.
> * Introduce a new `apache.aurora.tools` package with the `thermos` and 
> `thermos_observer` binaries.
> * Create apache.thermos.runner package, rename `thermos/bin` to 
> `thermos/runner`.
> * Remove all `*-packaged` and virtual dependency trees, as well as the phrase 
> "covering dependencies."
> * Use `_`-prefix naming convention for private target names.
> * Replace manual list of targets in `make-python-sdists` with 
> automatically-generated one (using new convention).
> * Introduce a new `apache.aurora.kerberos` package with the `kaurora` and 
> `kaurora_admin` binaries.
> * Remove all `BUILD` files in `src/main` that don't contain an exported 
> `setup.py` library (except one mentioned in TODO).
> * Use dictionary syntax in `with_binaries` (TODO: replace the underscores in 
> binary names with dashes?).
> * Remove unused/unreachable binaries (most `bin/` dirs under `thermos/`).
> * Create blank `__init__.py` files in directories that pants warns aren't 
> packages.
> * Added `thermos` command to vagrant image.
> 
> * TODO: Write NEWS entry
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> d196fefc2c5e5ee32d0cf9c901cffe7d247379d1 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> d0d789a6ee3971e3070f9397d53929563a77f7ea 
>   build-support/packaging/debian/rules 
> 17e00c02a3cb3294d5107516d795a73587ca4f70 
>   build-support/packaging/rpm/aurora.spec 
> 7cf8de6f7b99788ca461a90fc4aefba7dccd7b63 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
>   examples/vagrant/aurorabuild.sh fbaa6ae9ef7ff2910af8c9c0d6b8ef90ea3e152a 
>   src/main/python/apache/aurora/admin/BUILD 
> 22bf3f9943ea11258ba681bdb80feb00206bb926 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> 1c39717b656465bb1966073340f9fe80be01a085 
>   src/main/python/apache/aurora/client/BUILD 
> e73cd52289209bb9658b16bb77dc0b0a9c811a1a 
>   src/main/python/apache/aurora/client/api/BUILD 
> a030a67b78fc4bc4682d0df169e27efc6810dce3 
>   src/main/python/apache/aurora/client/cli/BUILD 
> 973d05971bd940c7e38c48ce6bfbf5c8e1654c5a 
>   src/main/python/apache/aurora/client/cli/client.py 
> c0974f3bef59f8f7c2320398d367cb4dd9048d2b 
>   src/main/python/apache/aurora/client/hooks/BUILD 
> ddf813b5d2e7d63507a8e08745ebf6cb3dbac8e1 
>   src/main/python/apache/aurora/common/BUILD 
> abc122b0775bb17b1df67bdb946c472010219b9b 
>   src/main/python/apache/aurora/common/auth/BUILD 
> 0abac94eeeb71e6af43ed191ea690e5f96a6be23 
>   src/main/python/apache/aurora/common/auth/kerberos.py 
> 2d782b63b611a9d2604ef0ab1116d3e68fb86dc7 
>   src/main/python/apache/aurora/config/BUILD 
> 0a3a93fa6bb785903f71bc067aecc79c0e45a0b5 
>   src/main/python/apache/aurora/config/schema/BUILD 
> 171f42a24ec7ddc2846eb68b6a60e7d8dec43

Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney


> On July 31, 2015, 10:19 a.m., Brian Wickman wrote:
> > src/main/python/apache/aurora/common/BUILD, line 61
> > <https://reviews.apache.org/r/36972/diff/2/?file=1025868#file1025868line61>
> >
> > This is probably the toughest one to swallow.  Pulling in aurora.common 
> > just for aurora_job_key now requires a native dependency (kerberos via 
> > requests-kerberos.)
> > 
> > I'd rather see auth get pulled into a top-level package (i.e. 
> > aurora.auth) than see it taint aurora.common with a number of heavyweight 
> > dependencies.

I've extracted an `apache.aurora.kerberos` package that provides the Kerberized 
client and admin client and moved the Kerberos dependency there.


- Kevin


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


On July 31, 2015, 10:56 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36972/
> ---
> 
> (Updated July 31, 2015, 10:56 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Publishing this review now for early feedback, don't intend to ship until 
> most of the TODOs are addressed.
> 
> This is a mostly red code diff and makes the pants build work similar to how 
> it already works in an IDE environment while minimizing duplicated 
> information. This change also removes the confusing versions of `thermos` and 
> `thermos_observer` in favor of new ones. Because of the way "covering 
> dependencies" were required it was easier to do this refactor than more 
> tactically create a working `setup_py` target for `apache.aurora.tools` 
> without cycles and without duplicated files.
> 
> * Remove the `apache.thermos` package.
> * Rename the `apache.gen.aurora` package to `apache.aurora.thrift`.
> * Rename the `apache.gen.thermos` package to `apache.thermos.thrift`.
> * Introduce a new `apache.aurora.tools` package with the `thermos` and 
> `thermos_observer` binaries.
> * Create apache.thermos.runner package, rename `thermos/bin` to 
> `thermos/runner`.
> * Remove all `*-packaged` and virtual dependency trees, as well as the phrase 
> "covering dependencies."
> * Use `_`-prefix naming convention for private target names.
> * Replace manual list of targets in `make-python-sdists` with 
> automatically-generated one (using new convention).
> * Introduce a new `apache.aurora.kerberos` package with the `kaurora` and 
> `kaurora_admin` binaries.
> * Remove all `BUILD` files in `src/main` that don't contain an exported 
> `setup.py` library (except one mentioned in TODO).
> * Use dictionary syntax in `with_binaries` (TODO: replace the underscores in 
> binary names with dashes?).
> * Remove unused/unreachable binaries (most `bin/` dirs under `thermos/`).
> * Create blank `__init__.py` files in directories that pants warns aren't 
> packages.
> * Added `thermos` command to vagrant image.
> 
> * TODO: Write NEWS entry
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/BUILD 
> d196fefc2c5e5ee32d0cf9c901cffe7d247379d1 
>   api/src/main/thrift/org/apache/thermos/BUILD 
> d0d789a6ee3971e3070f9397d53929563a77f7ea 
>   build-support/packaging/debian/rules 
> 17e00c02a3cb3294d5107516d795a73587ca4f70 
>   build-support/packaging/rpm/aurora.spec 
> 7cf8de6f7b99788ca461a90fc4aefba7dccd7b63 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
>   examples/vagrant/aurorabuild.sh fbaa6ae9ef7ff2910af8c9c0d6b8ef90ea3e152a 
>   src/main/python/apache/aurora/admin/BUILD 
> 22bf3f9943ea11258ba681bdb80feb00206bb926 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> 1c39717b656465bb1966073340f9fe80be01a085 
>   src/main/python/apache/aurora/client/BUILD 
> e73cd52289209bb9658b16bb77dc0b0a9c811a1a 
>   src/main/python/apache/aurora/client/api/BUILD 
> a030a67b78fc4bc4682d0df169e27efc6810dce3 
>   src/main/python/apache/aurora/client/cli/BUILD 
> 973d05971bd940c7e38c48ce6bfbf5c8e1654c5a 
>   src/main/python/apache/aurora/client/cli/client.py 
> c0974f3bef59f8f7c2320398d367cb4dd9048d2b 
>   src/main/python/apache/aurora/client/hooks/BUILD 
> ddf813b5d2e7d63507a8e08745ebf6cb3dbac8e1 
>   src/main/python/apache/aurora/common/BUILD

Re: Review Request 36972: One python_library per exported setup.py project

2015-07-31 Thread Kevin Sweeney
 src/main/python/apache/thermos/cli/bin/thermos.py 
fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
  src/main/python/apache/thermos/cli/commands/BUILD 
552eeb4f61693033d8828e789bd22e680b957d1d 
  src/main/python/apache/thermos/common/BUILD 
942fc15f5a5d6a5ff58385b10f3783ac476a4f82 
  src/main/python/apache/thermos/config/BUILD 
d9099c5806eadb1190d2028142a7ec711023d39f 
  src/main/python/apache/thermos/config/bin/config_load.py 
d6e1f820ac6b0fa4e47e26f99df1602a2d3d021e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists

E2E test gets as far as it usually does.
Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.


Thanks,

Kevin Sweeney



Re: Review Request 36972: One python_library per exported setup.py project

2015-07-30 Thread Kevin Sweeney
e 
  src/main/python/apache/thermos/config/bin/config_repl.py 
ae9ca3b2812f719b0a6185081434d557492ac825 
  src/main/python/apache/thermos/core/BUILD 
d47b7a2c5db8ba9a85e2403b7d6bf2dea3f045ea 
  src/main/python/apache/thermos/monitoring/BUILD 
e8851026cb7ff59f0fd719837fdb0110be356c31 
  src/main/python/apache/thermos/observer/BUILD 
41db2cc2e11234c434f58f55abec0b9f308096be 
  src/main/python/apache/thermos/observer/bin/BUILD 
0abe2ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing (updated)
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists

E2E test gets as far as it usually does.
Manually checked that `thermos status` works.

I don't have a test environment for the debian package or the spec file.


Thanks,

Kevin Sweeney



Review Request 36972: One python_library per exported setup.py project

2015-07-30 Thread Kevin Sweeney
ccfe9c5ccb509ad731125385900114ba352 
  src/main/python/apache/thermos/observer/bin/__init__.py  
  src/main/python/apache/thermos/observer/bin/thermos_observer.py 
39d3994a6163746e853cd21fc4c3585edc2b54cb 
  src/main/python/apache/thermos/observer/http/BUILD 
0bd770453dc78b043c9e6171dc7439da19c5872e 
  src/main/python/apache/thermos/testing/BUILD 
8b5f6dc93c95e2f69d1b755e93b5f24dec0ead30 
  src/test/python/apache/aurora/BUILD c2251ce4768b9bc9d4c4f6869bddcb23f0f6f986 
  src/test/python/apache/aurora/admin/BUILD 
69da2c97cb08025a27ca276bb2ad6fcc43db1b10 
  src/test/python/apache/aurora/client/BUILD 
c55adfe9825b77f418e41fa9a4ba43926bd991ed 
  src/test/python/apache/aurora/client/api/BUILD 
65b378b0b4c5fa11f9899ef04a4a10a211f37245 
  src/test/python/apache/aurora/client/cli/BUILD 
0d85f5fba9d19ae0e9c36546f130b93664b4f6de 
  src/test/python/apache/aurora/client/hooks/BUILD 
0b924ccfbcdb4800f99b80067ff8a78252f99907 
  src/test/python/apache/aurora/common/BUILD 
9b1d4d876e10cb759202240931e4787ce673d897 
  src/test/python/apache/aurora/config/BUILD 
c85e998e66a989bb3e2ad8b9aab03702f33c0ef9 
  src/test/python/apache/aurora/executor/BUILD 
8fff66ef8858af892ba1124454fefb07715943d7 
  src/test/python/apache/aurora/executor/bin/BUILD 
713ce9529e36c70e1dde17a724efe7631c7aca4c 
  src/test/python/apache/aurora/executor/common/BUILD 
b3da27bb83c44a228851b256cc09f093d1e2eb87 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
c3baa9f4b9bfbfa3ab4eb8ab4d74d49c49b6b057 
  src/test/python/apache/aurora/executor/test_thermos_task_runner.py 
cc326562db59eee807b22c4ee16d753abb3c6468 
  src/test/python/apache/thermos/cli/BUILD 
087b1740de4d8a30c74c7776eb83f9784e0049cc 
  src/test/python/apache/thermos/cli/commands/BUILD 
5465b19be5ab8bdf7252e7b1fa7a4ef95063193d 
  src/test/python/apache/thermos/common/BUILD 
bb70867e0e070ee9651f018f30990bd9ff0dd88f 
  src/test/python/apache/thermos/config/BUILD 
a06af36c83fc7ec00ecd00df7e424faae3118131 
  src/test/python/apache/thermos/core/BUILD 
d637c24f098468776fb8b0758f5990b83ec362c5 
  src/test/python/apache/thermos/monitoring/BUILD 
f4ad7fc1245980ef727c7c4a30af89b2d8d1293a 
  src/test/python/apache/thermos/observer/BUILD 
ff92a52fe7a88379e8aa1c1b2385c065bd375a68 
  src/test/python/apache/thermos/observer/http/BUILD 
0cdaafc3e71d21d48c35e0dac68910c53d003fae 

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


Testing
---

./pants test.pytest --no-fast src/test/python::
./build-support/python/make-python-sdists

TODO: test RPMs
TODO: test DEBs
TODO: test E2E


Thanks,

Kevin Sweeney



Re: Review Request 36945: Use the correct (aurora-specific) python build targets in RPM and deb scripts.

2015-07-30 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On July 30, 2015, 9:13 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36945/
> ---
> 
> (Updated July 30, 2015, 9:13 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Pulled this out of https://reviews.apache.org/r/36700/ to address separately, 
> as it is the more pressing issue.
> 
> 
> Diffs
> -
> 
>   build-support/packaging/debian/rules 
> 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 
> 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
> 
> Diff: https://reviews.apache.org/r/36945/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Review Request 36874: Fix typo in the scheduler deployment documentation

2015-07-27 Thread Kevin Sweeney

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

Review request for Aurora and Maxim Khutornenko.


Repository: aurora


Description
---

Fix typo in scheduler deployment documentation

The command should be run on schedulers, not masters.


Diffs
-

  docs/deploying-aurora-scheduler.md 11155b9495b6e3ab59038b34e5e4deab59f7172d 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

2015-07-27 Thread Kevin Sweeney

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



README.md 
<https://reviews.apache.org/r/36700/#comment147530>

Is there any reason to remove the `./gradlew distTar` guidance?


- Kevin Sweeney


On July 27, 2015, 5:58 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> ---
> 
> (Updated July 27, 2015, 5:58 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The meaning of the targets under 
> `src/main/python/apache/thermos/cli/bin/BUILD` and 
> `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which 
> bit several people (including our own packaging).  This removes targets that 
> Aurora users should not be consuming.
> 
> 
> Diffs
> -
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 
> 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 
> 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 36700: Remove references to binary build targets that are not currently for general consumption.

2015-07-27 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On July 27, 2015, 5:58 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> ---
> 
> (Updated July 27, 2015, 5:58 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The meaning of the targets under 
> `src/main/python/apache/thermos/cli/bin/BUILD` and 
> `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which 
> bit several people (including our own packaging).  This removes targets that 
> Aurora users should not be consuming.
> 
> 
> Diffs
> -
> 
>   README.md f3b23247cf3f7e550c1714b4fb8227a2baab3b41 
>   build-support/jenkins/build.sh 5606bb157cb117a588f363382d7c8841ae957138 
>   build-support/packaging/debian/rules 
> 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 
> 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   build-support/release/make-python-sdists 
> 9608f68e16243da01434ce2fc7d61bb7c7efd712 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 36700: Remove binary build targets that are not currently for general consumption.

2015-07-27 Thread Kevin Sweeney


> On July 24, 2015, 10:57 a.m., Brian Wickman wrote:
> > src/main/python/apache/thermos/observer/BUILD, line 74
> > <https://reviews.apache.org/r/36700/diff/2/?file=1020365#file1020365line74>
> >
> > This puts an aurora dependency on thermos and I imagine causes a cycle 
> > in the build graph (pants may just silently ignore it.)
> > 
> > Let's just remove the thermos_observer entry point from this package 
> > entirely by deleting the .with_binaries clause.
> 
> Bill Farner wrote:
> This target (`src/main/python/apache/thermos/observer`) is a top-level 
> target, only used by `build-support/release/make-python-sdists`, so i think 
> we're safe from a cycle.  I'm not strictly against removing the 
> `with_binaries`, but i assume it is there for a reason and something would 
> break?  Or do we just not use the sdists [this way]?
> 
> Brian Wickman wrote:
> Afaik nobody uses these in practice.  They're only useful for when you 
> install the sdists via pip or pex directly.  Instead we always build pex 
> binaries using pants which delegates to the python_binary target.
> 
> Kevin Sweeney wrote:
> I answered someone's question in IRC with a gist ~2yrs ago 
> https://gist.github.com/kevints/8361414 so it does seem likely these are used 
> in practice. Okay with removing them but a NEWS entry would be good form.
> 
> Bill Farner wrote:
> Should i just kill the sdists entirely?
> 
> Brian Wickman wrote:
> I'd prefer to go in the opposite direction and kill pants.
> 
> Bill Farner wrote:
> I'd like to find a path to land a fix as packaging is currently 
> broken/blocked.  I believe the current diff is the closest thing to 
> compromise between the various opinions around this change.  Please let me 
> know what you think.

-1 as this fix breaks the observer entry point entirely and produces an sdist 
that is invalid

```
(observer.venv)~aurora git aurora/. 36700
% pip install -f dist apache.thermos.observer --pre   
You are using pip version 6.1.1, however version 7.1.0 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting apache.thermos.observer
  Could not find a version that satisfies the requirement 
apache.thermos.observer (from versions: 0.10.0-SNAPSHOT)
  No matching distribution found for apache.thermos.observer
```

so pants writes this sdist with a dependency on itself

since it's already broken I'd suggest removing the entry point entirely (so 
we're not advertising a feature we don't actually have).


- Kevin


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


On July 23, 2015, 5 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36700/
> ---
> 
> (Updated July 23, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-1381
> https://issues.apache.org/jira/browse/AURORA-1381
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The meaning of the targets under 
> `src/main/python/apache/thermos/cli/bin/BUILD` and 
> `src/main/python/apache/thermos/observer/bin/BUILD` changed recently, which 
> bit several people (including our own packaging).  This removes targets that 
> Aurora users should not be consuming.
> 
> 
> Diffs
> -
> 
>   build-support/packaging/debian/rules 
> 23828c02b73f007393d2ed4ce69c010cebf07e57 
>   build-support/packaging/rpm/aurora.spec 
> 0c4c106a1730ee7e4e62c7ba778e7f0e2cb44771 
>   src/main/python/apache/thermos/BUILD 
> 8221aa0bd4efe5f519550cba716d6a564ba9ae44 
>   src/main/python/apache/thermos/cli/bin/BUILD 
> f33c7f838a6a0932abc737d0ecf7ca3a59580e2e 
>   src/main/python/apache/thermos/cli/bin/thermos.py 
> fcef38d33e7ca2005f35c3bdb90ffee6aeade3af 
>   src/main/python/apache/thermos/observer/BUILD 
> 41db2cc2e11234c434f58f55abec0b9f308096be 
>   src/main/python/apache/thermos/observer/bin/BUILD 
> 0abe2ccfe9c5ccb509ad731125385900114ba352 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> 39d3994a6163746e853cd21fc4c3585edc2b54cb 
> 
> Diff: https://reviews.apache.org/r/36700/diff/
> 
> 
> Testing
> ---
> 
> ./build-support/jenkins/build.sh
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> (as far as it currently gets on master, anyhow, due to AURORA-1409)
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 36710: Add an executor service decorator that gates async operations.

2015-07-24 Thread Kevin Sweeney

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

Ship it!


Ship It!

- Kevin Sweeney


On July 23, 2015, 10:20 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36710/
> ---
> 
> (Updated July 23, 2015, 10:20 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-1395
> https://issues.apache.org/jira/browse/AURORA-1395
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> In the current state of the scheduler, we start async work in the context of 
> a transaction.  As we have observed in the linked ticket, this work can read 
> data that is inconsistent with the transaction context if it starts before 
> the transaction completes.  The purpose of this utility is to let async work 
> queue until the originating transaction completes.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/DelayExecutor.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/FlushableWorkQueue.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/async/GatedDelayExecutor.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> c85979dedd0ef2c515453a33c9a36d52865eb548 
>   src/test/java/org/apache/aurora/scheduler/async/GatedDelayExecutorTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36710/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



  1   2   3   >