Re: Review Request 41074: Use lambdas throughout the project.

2015-12-07 Thread Aurora ReviewBot

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

Ship it!


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

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

- Aurora ReviewBot


On Dec. 8, 2015, 5:08 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41074/
> ---
> 
> (Updated Dec. 8, 2015, 5:08 a.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is almost entirely IDE-assisted.  There were some spots that required 
> hand-editing (notably casting the argument to `storage.write`).
> 
> The overall stats are nice, a reduction of about 1300 lines:
> ```
>  114 files changed, 1982 insertions(+), 3292 deletions(-)
> ```
> 
> 
> Diffs
> -
> 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
>  5abb06b1d41f3265d9d46c600ba193ce19d21188 
>   
> commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
>  d06ec69cb57d9c411ae33b301489f6460197dcc7 
>   commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
> 9a4d441023fd9eea868fd069c197eb002d98f0ab 
>   commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
> cc4710cb5f559a108b140489a981a9d2c195b75b 
>   commons/src/main/java/org/apache/aurora/common/args/Args.java 
> ad0b299057e787a9587822e7778bd360f77e1678 
>   commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
> 051e3f93ef17de90b7bffaa74fc6b54965927424 
>   commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
> c1fe1c73c46da21c471b2669dbb8207b7c468054 
>   commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
> 62564a62d255e640e5790ed2e103ba8425a4c15b 
>   commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java 
> 3da1812abfdb30f12737b19f5e87bc14a02d4c3f 
>   
> commons/src/main/java/org/apache/aurora/common/args/constraints/RangeNumberVerifier.java
>  27250944a60f906a4c220771e9948ed0297ee76c 
>   commons/src/main/java/org/apache/aurora/common/args/parsers/ListParser.java 
> 02c10ed067c8395a2688cd69d4d73d5fd17774ff 
>   commons/src/main/java/org/apache/aurora/common/args/parsers/SetParser.java 
> 1e12b06e284cd2e33b63b3690abd3dcd78f9e777 
>   commons/src/main/java/org/apache/aurora/common/base/Closures.java 
> d6cb82a30a8c6292bab0fa007dce2f54f6b66435 
>   commons/src/main/java/org/apache/aurora/common/base/Commands.java 
> 8a88ae336f57fcdafb984cc3418dcd960da0028d 
>   commons/src/main/java/org/apache/aurora/common/collections/Iterables2.java 
> 072159e79b2d8872f39dfbba9f6a1d27295f1a7a 
>   commons/src/main/java/org/apache/aurora/common/collections/Pair.java 
> b1f9b6336aa8b8d0ff9e611dc2ad1dc8068ef7f3 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/LogConfig.java
>  348a8c923118da3b4399afb1a6f09d904c3b 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/TimeSeriesDataSource.java
>  8039def73377b0e56c3106876dc89e9183cd55a1 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsHandler.java
>  d1f23de1a440d47a7754af2c8418e2c4e96bbb84 
>   commons/src/main/java/org/apache/aurora/common/stats/JvmStats.java 
> 03df9b6f7d31d8567a6bd135654eb39026c375d7 
>   commons/src/main/java/org/apache/aurora/common/stats/Rate.java 
> ff29c39863f044db7fde0fff315621644393429d 
>   commons/src/main/java/org/apache/aurora/common/stats/Ratio.java 
> c332dd3af5712bfb8ed174c2285cac4d30394acc 
>   
> commons/src/main/java/org/apache/aurora/common/testing/easymock/EasyMockTest.java
>  403147527eab46e9c23ab5690d885a6c1deb1c30 
>   commons/src/main/java/org/apache/aurora/common/util/BackoffHelper.java 
> e789f804ecc0053d4605927dc981311af1df94c8 
>   commons/src/main/java/org/apache/aurora/common/util/StateMachine.java 
> 9fbfbb9012414a19cf752d69b48f2e937f4497e7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 
> b10a40327a062b354b42249097c89bf3fbecb1e8 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 
> 99a15fe8d58138ebd264d858bff842316928db06 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java 
> bb6fbced0adc0920592ec14f03a84895829d8812 
>   
> commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java 
> 1761ab70a32656af13a311bcc02f716fdc0ad067 
>   config/legacy_untested_classes.txt 07e49b11050f8b9be63d5b95cfcdb7800e15c7e5 
>   src/main/java/org/apache/aurora/GuiceUtils.java 
> 78b60c26231ff070f08f236f38c8fb58ca9f8d60 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> 7003b2f0b73bf050ae243da4da4c8

Review Request 41074: Use lambdas throughout the project.

2015-12-07 Thread Bill Farner

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

Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

This is almost entirely IDE-assisted.  There were some spots that required 
hand-editing (notably casting the argument to `storage.write`).

The overall stats are nice, a reduction of about 1300 lines:
```
 114 files changed, 1982 insertions(+), 3292 deletions(-)
```


Diffs
-

  
commons-args/src/main/java/org/apache/aurora/common/args/apt/CmdLineProcessor.java
 5abb06b1d41f3265d9d46c600ba193ce19d21188 
  
commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java 
d06ec69cb57d9c411ae33b301489f6460197dcc7 
  commons/src/main/java/org/apache/aurora/common/args/ArgFilters.java 
9a4d441023fd9eea868fd069c197eb002d98f0ab 
  commons/src/main/java/org/apache/aurora/common/args/ArgScanner.java 
cc4710cb5f559a108b140489a981a9d2c195b75b 
  commons/src/main/java/org/apache/aurora/common/args/Args.java 
ad0b299057e787a9587822e7778bd360f77e1678 
  commons/src/main/java/org/apache/aurora/common/args/ArgumentInfo.java 
051e3f93ef17de90b7bffaa74fc6b54965927424 
  commons/src/main/java/org/apache/aurora/common/args/OptionInfo.java 
c1fe1c73c46da21c471b2669dbb8207b7c468054 
  commons/src/main/java/org/apache/aurora/common/args/Parsers.java 
62564a62d255e640e5790ed2e103ba8425a4c15b 
  commons/src/main/java/org/apache/aurora/common/args/PositionalInfo.java 
3da1812abfdb30f12737b19f5e87bc14a02d4c3f 
  
commons/src/main/java/org/apache/aurora/common/args/constraints/RangeNumberVerifier.java
 27250944a60f906a4c220771e9948ed0297ee76c 
  commons/src/main/java/org/apache/aurora/common/args/parsers/ListParser.java 
02c10ed067c8395a2688cd69d4d73d5fd17774ff 
  commons/src/main/java/org/apache/aurora/common/args/parsers/SetParser.java 
1e12b06e284cd2e33b63b3690abd3dcd78f9e777 
  commons/src/main/java/org/apache/aurora/common/base/Closures.java 
d6cb82a30a8c6292bab0fa007dce2f54f6b66435 
  commons/src/main/java/org/apache/aurora/common/base/Commands.java 
8a88ae336f57fcdafb984cc3418dcd960da0028d 
  commons/src/main/java/org/apache/aurora/common/collections/Iterables2.java 
072159e79b2d8872f39dfbba9f6a1d27295f1a7a 
  commons/src/main/java/org/apache/aurora/common/collections/Pair.java 
b1f9b6336aa8b8d0ff9e611dc2ad1dc8068ef7f3 
  
commons/src/main/java/org/apache/aurora/common/net/http/handlers/LogConfig.java 
348a8c923118da3b4399afb1a6f09d904c3b 
  
commons/src/main/java/org/apache/aurora/common/net/http/handlers/TimeSeriesDataSource.java
 8039def73377b0e56c3106876dc89e9183cd55a1 
  
commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsHandler.java
 d1f23de1a440d47a7754af2c8418e2c4e96bbb84 
  commons/src/main/java/org/apache/aurora/common/stats/JvmStats.java 
03df9b6f7d31d8567a6bd135654eb39026c375d7 
  commons/src/main/java/org/apache/aurora/common/stats/Rate.java 
ff29c39863f044db7fde0fff315621644393429d 
  commons/src/main/java/org/apache/aurora/common/stats/Ratio.java 
c332dd3af5712bfb8ed174c2285cac4d30394acc 
  
commons/src/main/java/org/apache/aurora/common/testing/easymock/EasyMockTest.java
 403147527eab46e9c23ab5690d885a6c1deb1c30 
  commons/src/main/java/org/apache/aurora/common/util/BackoffHelper.java 
e789f804ecc0053d4605927dc981311af1df94c8 
  commons/src/main/java/org/apache/aurora/common/util/StateMachine.java 
9fbfbb9012414a19cf752d69b48f2e937f4497e7 
  commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 
b10a40327a062b354b42249097c89bf3fbecb1e8 
  commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 
99a15fe8d58138ebd264d858bff842316928db06 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java 
bb6fbced0adc0920592ec14f03a84895829d8812 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java 
1761ab70a32656af13a311bcc02f716fdc0ad067 
  config/legacy_untested_classes.txt 07e49b11050f8b9be63d5b95cfcdb7800e15c7e5 
  src/main/java/org/apache/aurora/GuiceUtils.java 
78b60c26231ff070f08f236f38c8fb58ca9f8d60 
  src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
7003b2f0b73bf050ae243da4da4c844e77cdb04b 
  src/main/java/org/apache/aurora/scheduler/Resources.java 
f7d105eca961da2b5b445d4a0670e3d5e9bf89b2 
  src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 
29331e075b29f0994b4ce4dc8ce602b01b307f0a 
  src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
6d940d17fcb95a221d7d125cc13423e384e4144e 
  src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java 
311257921df7dca9c2002fbf4c8569d297207c58 
  src/main/java/org/apache/aurora/scheduler/TaskVars.java 
da9f22279a4cd1005a85e8b78398ed57ac6cac34 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
54814b2b104bb0140d73b314f70ed42d9e0ec454 
  src/main/java/org/apache/aurora/scheduler/base/Conver

Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Aurora ReviewBot

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

Ship it!


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

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

- Aurora ReviewBot


On Dec. 8, 2015, 3:46 a.m., Tengfei Mu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41048/
> ---
> 
> (Updated Dec. 8, 2015, 3:46 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1547
> https://issues.apache.org/jira/browse/AURORA-1547
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> set TaskStatus.slave_id field in TaskReconsiler
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
> c797914ded51cd123dddef188373fe58e190773a 
>   
> src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
>  1308a1c9884e7759e6139787710b367bbf9cd4e9 
> 
> Diff: https://reviews.apache.org/r/41048/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Tengfei Mu
> 
>



Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Tengfei Mu

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

(Updated Dec. 8, 2015, 3:46 a.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

address maxim's comment


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


Repository: aurora


Description
---

set TaskStatus.slave_id field in TaskReconsiler


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
c797914ded51cd123dddef188373fe58e190773a 
  
src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
 1308a1c9884e7759e6139787710b367bbf9cd4e9 

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


Testing
---

./gradlew -Pq build


Thanks,

Tengfei Mu



Re: Review Request 40889: Changed mesos native lib to use mesos.executor instead

2015-12-07 Thread Steve Niemitz


> On Dec. 3, 2015, 12:36 a.m., Maxim Khutornenko wrote:
> > 3rdparty/python/BUILD, line 14
> > 
> >
> > Am I reading this as revving up mesos native version in Aurora? If so, 
> > we should also update build.gradle and possibly update 
> > make-mesos-native-egg script. Previous effort as a reference: 
> > https://reviews.apache.org/r/37379
> 
> Bill Farner wrote:
> You're right, Maxim.  I think it's safe to say this patch is still in PoC 
> stage, as it can't land until some changes go into mesos.  I think Steve is 
> not expecting ship-its just yet.
> 
> Steve Niemitz wrote:
> Correct, I'll ping when I'm ready for more eyes on it.
> 
> Zameer Manji wrote:
> If you do want to do this, please assign 
> https://issues.apache.org/jira/browse/AURORA-1543 to yourself.
> 
> Steve Niemitz wrote:
> The version number here is pretty much a placeholder, what it ends up 
> being is going to depend on how we want to support the mesos native library.  
> I see a couple options (and maybe there's others):
> 1. Wait for my patch (which I'm about to submit (1)) to land in mesos and 
> let the normal aurora->mesos upgrade cycle get it.  It will be at least 
> 0.26.0 in this case, as 0.25.0 is already released.
> 2. Host our own builds of mesos.executor somewhere, and download/use that 
> when doing the thermos build.  We could build it off of pretty much any mesos 
> version, including the current 0.23.0.
> 
> Seeing as mesos proper doesn't provide debs/rpms anyways (mesospehere 
> does instead), and the same mesos.executor egg *should* work in either centos 
> or ubuntu, option 2 might make the most sense, especially if we want to get 
> this out sometime soon.
> 
> (1): https://issues.apache.org/jira/browse/MESOS-4090
> 
> Bill Farner wrote:
> I could be convinced of (2), so long as we can point to the sha we built 
> from + patch we applied.

Thoughts on where would you want to host it from?  It looks like the current 
native egg is downloaded from svn.apache.org for the vagrant build, would we 
want to follow the same pattern?

I could supply a dockerfile to build it in aurora/build-support as well.


- Steve


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


On Dec. 2, 2015, 11:57 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40889/
> ---
> 
> (Updated Dec. 2, 2015, 11:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changed mesos native lib to use mesos.executor instead
> 
> Without any context this doesnt really make any sense.
> 
> I've created a seperate mesos native lib that only contains a minimal set of 
> code needed to run the ExecutorDriver.  This library has very few 
> requirements, making it a much better option for running in docker 
> containers, as well as allowing a thermos_executor.pex built on one linux 
> distro to run on most others.
> 
> The mesos side of this is here: 
> https://github.com/tellapart/mesos/commit/46bedd5db73b535a0e4efe94730500b6da2ec7d8
> and I'll be opening a RB request on the mesos side soon.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 
>   src/main/python/apache/aurora/executor/BUILD 
> 0be65fc7c180d15f5a94bef268652ef7b868dcf7 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
> 
> Diff: https://reviews.apache.org/r/40889/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>



Re: Review Request 41056: Upgrade checkstyle and pmd versions.

2015-12-07 Thread Aurora ReviewBot

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


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

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Dec. 7, 2015, 10:56 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41056/
> ---
> 
> (Updated Dec. 7, 2015, 10:56 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We've been holding back on PMD upgades for a while.  This was due to false 
> positives, but they seem to have been resolved.  With these updates, i 
> believe we can make more extensive use of lambdas without tripping over QA 
> tools.
> 
> Release notes:
> https://pmd.github.io/pmd-5.4.1/overview/changelog.html
> http://checkstyle.sourceforge.net/releasenotes.html
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   config/pmd/custom.xml c31cac98b14960057905d5224aa23824e48ccf42 
>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
> 3709c775f80f84d9c4df72d2414c24bf8f6a802b 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> ea43912380f89b4fb06d73c63ec23c47b8f47288 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  326d4fbc3261c57c44d4dc51d74e05b1155de6ad 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> 0acceaaec851779809adc339a441f33661f9308c 
> 
> Diff: https://reviews.apache.org/r/41056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 41056: Upgrade checkstyle and pmd versions.

2015-12-07 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Dec. 7, 2015, 2:56 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41056/
> ---
> 
> (Updated Dec. 7, 2015, 2:56 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> We've been holding back on PMD upgades for a while.  This was due to false 
> positives, but they seem to have been resolved.  With these updates, i 
> believe we can make more extensive use of lambdas without tripping over QA 
> tools.
> 
> Release notes:
> https://pmd.github.io/pmd-5.4.1/overview/changelog.html
> http://checkstyle.sourceforge.net/releasenotes.html
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   config/pmd/custom.xml c31cac98b14960057905d5224aa23824e48ccf42 
>   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
> 3709c775f80f84d9c4df72d2414c24bf8f6a802b 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> ea43912380f89b4fb06d73c63ec23c47b8f47288 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  326d4fbc3261c57c44d4dc51d74e05b1155de6ad 
>   src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
> 0acceaaec851779809adc339a441f33661f9308c 
> 
> Diff: https://reviews.apache.org/r/41056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Review Request 41056: Upgrade checkstyle and pmd versions.

2015-12-07 Thread Bill Farner

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

Review request for Aurora and Zameer Manji.


Repository: aurora


Description
---

We've been holding back on PMD upgades for a while.  This was due to false 
positives, but they seem to have been resolved.  With these updates, i believe 
we can make more extensive use of lambdas without tripping over QA tools.

Release notes:
https://pmd.github.io/pmd-5.4.1/overview/changelog.html
http://checkstyle.sourceforge.net/releasenotes.html


Diffs
-

  build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
  config/pmd/custom.xml c31cac98b14960057905d5224aa23824e48ccf42 
  src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
3709c775f80f84d9c4df72d2414c24bf8f6a802b 
  src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
ea43912380f89b4fb06d73c63ec23c47b8f47288 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
326d4fbc3261c57c44d4dc51d74e05b1155de6ad 
  src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 
0acceaaec851779809adc339a441f33661f9308c 

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


Testing
---


Thanks,

Bill Farner



Re: Review Request 40889: Changed mesos native lib to use mesos.executor instead

2015-12-07 Thread Bill Farner


> On Dec. 2, 2015, 4:36 p.m., Maxim Khutornenko wrote:
> > 3rdparty/python/BUILD, line 14
> > 
> >
> > Am I reading this as revving up mesos native version in Aurora? If so, 
> > we should also update build.gradle and possibly update 
> > make-mesos-native-egg script. Previous effort as a reference: 
> > https://reviews.apache.org/r/37379
> 
> Bill Farner wrote:
> You're right, Maxim.  I think it's safe to say this patch is still in PoC 
> stage, as it can't land until some changes go into mesos.  I think Steve is 
> not expecting ship-its just yet.
> 
> Steve Niemitz wrote:
> Correct, I'll ping when I'm ready for more eyes on it.
> 
> Zameer Manji wrote:
> If you do want to do this, please assign 
> https://issues.apache.org/jira/browse/AURORA-1543 to yourself.
> 
> Steve Niemitz wrote:
> The version number here is pretty much a placeholder, what it ends up 
> being is going to depend on how we want to support the mesos native library.  
> I see a couple options (and maybe there's others):
> 1. Wait for my patch (which I'm about to submit (1)) to land in mesos and 
> let the normal aurora->mesos upgrade cycle get it.  It will be at least 
> 0.26.0 in this case, as 0.25.0 is already released.
> 2. Host our own builds of mesos.executor somewhere, and download/use that 
> when doing the thermos build.  We could build it off of pretty much any mesos 
> version, including the current 0.23.0.
> 
> Seeing as mesos proper doesn't provide debs/rpms anyways (mesospehere 
> does instead), and the same mesos.executor egg *should* work in either centos 
> or ubuntu, option 2 might make the most sense, especially if we want to get 
> this out sometime soon.
> 
> (1): https://issues.apache.org/jira/browse/MESOS-4090

I could be convinced of (2), so long as we can point to the sha we built from + 
patch we applied.


- Bill


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


On Dec. 2, 2015, 3:57 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40889/
> ---
> 
> (Updated Dec. 2, 2015, 3:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changed mesos native lib to use mesos.executor instead
> 
> Without any context this doesnt really make any sense.
> 
> I've created a seperate mesos native lib that only contains a minimal set of 
> code needed to run the ExecutorDriver.  This library has very few 
> requirements, making it a much better option for running in docker 
> containers, as well as allowing a thermos_executor.pex built on one linux 
> distro to run on most others.
> 
> The mesos side of this is here: 
> https://github.com/tellapart/mesos/commit/46bedd5db73b535a0e4efe94730500b6da2ec7d8
> and I'll be opening a RB request on the mesos side soon.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 
>   src/main/python/apache/aurora/executor/BUILD 
> 0be65fc7c180d15f5a94bef268652ef7b868dcf7 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
> 
> Diff: https://reviews.apache.org/r/40889/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>



Re: Review Request 40889: Changed mesos native lib to use mesos.executor instead

2015-12-07 Thread Steve Niemitz


> On Dec. 3, 2015, 12:36 a.m., Maxim Khutornenko wrote:
> > 3rdparty/python/BUILD, line 14
> > 
> >
> > Am I reading this as revving up mesos native version in Aurora? If so, 
> > we should also update build.gradle and possibly update 
> > make-mesos-native-egg script. Previous effort as a reference: 
> > https://reviews.apache.org/r/37379
> 
> Bill Farner wrote:
> You're right, Maxim.  I think it's safe to say this patch is still in PoC 
> stage, as it can't land until some changes go into mesos.  I think Steve is 
> not expecting ship-its just yet.
> 
> Steve Niemitz wrote:
> Correct, I'll ping when I'm ready for more eyes on it.
> 
> Zameer Manji wrote:
> If you do want to do this, please assign 
> https://issues.apache.org/jira/browse/AURORA-1543 to yourself.

The version number here is pretty much a placeholder, what it ends up being is 
going to depend on how we want to support the mesos native library.  I see a 
couple options (and maybe there's others):
1. Wait for my patch (which I'm about to submit (1)) to land in mesos and let 
the normal aurora->mesos upgrade cycle get it.  It will be at least 0.26.0 in 
this case, as 0.25.0 is already released.
2. Host our own builds of mesos.executor somewhere, and download/use that when 
doing the thermos build.  We could build it off of pretty much any mesos 
version, including the current 0.23.0.

Seeing as mesos proper doesn't provide debs/rpms anyways (mesospehere does 
instead), and the same mesos.executor egg *should* work in either centos or 
ubuntu, option 2 might make the most sense, especially if we want to get this 
out sometime soon.

(1): https://issues.apache.org/jira/browse/MESOS-4090


- Steve


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


On Dec. 2, 2015, 11:57 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40889/
> ---
> 
> (Updated Dec. 2, 2015, 11:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changed mesos native lib to use mesos.executor instead
> 
> Without any context this doesnt really make any sense.
> 
> I've created a seperate mesos native lib that only contains a minimal set of 
> code needed to run the ExecutorDriver.  This library has very few 
> requirements, making it a much better option for running in docker 
> containers, as well as allowing a thermos_executor.pex built on one linux 
> distro to run on most others.
> 
> The mesos side of this is here: 
> https://github.com/tellapart/mesos/commit/46bedd5db73b535a0e4efe94730500b6da2ec7d8
> and I'll be opening a RB request on the mesos side soon.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 
>   src/main/python/apache/aurora/executor/BUILD 
> 0be65fc7c180d15f5a94bef268652ef7b868dcf7 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
> 
> Diff: https://reviews.apache.org/r/40889/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>



Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.

2015-12-07 Thread Aurora ReviewBot

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

Ship it!


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

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

- Aurora ReviewBot


On Dec. 7, 2015, 9:55 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40786/
> ---
> 
> (Updated Dec. 7, 2015, 9:55 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a bit of a straw man.  While working on
> https://issues.apache.org/jira/browse/AURORA-987 I found the need for
> `javapoet` and this project spun off the side.
> 
> See the `@Forward` README here for more info: 
> https://github.com/perkuno/forward
> 
> Although I think the end result is desirable, this change does add a
> dependency on a personal project.  I fancied up the presentation with a
> custom org, but its still a personal project. I'd be happy enough to
> move the `@Forward` code over to aurora, but it would require a seperate
> gradle module to ensure the annotation processor is compiled ahead of
> the main module that would use it.
> 
> So kick the tires and let me know what you think.
> 
>  build.gradle 
>  |   6 +-
>  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java   
>  | 185 --
>  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
>  |  13 ++-
>  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java   
>  | 309 --
>  
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> |   5 +-
>  5 files changed, 20 insertions(+), 498 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> b8bd9185cacc6b113b64a13a1b670fac202c795e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2de178302d3c9aa9a7b23a9eb7ecb6e2f3b40819 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 1415f0cacc694aa7cf0d25e836e764a96fbb8ae2 
> 
> Diff: https://reviews.apache.org/r/40786/diff/
> 
> 
> Testing
> ---
> 
> Green locally `./build-support/jenkins/build.sh`.
> 
> An example of the generated code:
> ```java
> @Generated("uno.perk.forward.apt.Processor")
> class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
>   protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
> 
>   MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
> this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
>   }
> 
>   @Override
>   public Response getRoleSummary() throws TException {
> return this.annotatedAuroraAdmin.getRoleSummary();
>   }
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 40889: Changed mesos native lib to use mesos.executor instead

2015-12-07 Thread Zameer Manji


> On Dec. 2, 2015, 4:36 p.m., Maxim Khutornenko wrote:
> > 3rdparty/python/BUILD, line 14
> > 
> >
> > Am I reading this as revving up mesos native version in Aurora? If so, 
> > we should also update build.gradle and possibly update 
> > make-mesos-native-egg script. Previous effort as a reference: 
> > https://reviews.apache.org/r/37379
> 
> Bill Farner wrote:
> You're right, Maxim.  I think it's safe to say this patch is still in PoC 
> stage, as it can't land until some changes go into mesos.  I think Steve is 
> not expecting ship-its just yet.
> 
> Steve Niemitz wrote:
> Correct, I'll ping when I'm ready for more eyes on it.

If you do want to do this, please assign 
https://issues.apache.org/jira/browse/AURORA-1543 to yourself.


- Zameer


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


On Dec. 2, 2015, 3:57 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40889/
> ---
> 
> (Updated Dec. 2, 2015, 3:57 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Changed mesos native lib to use mesos.executor instead
> 
> Without any context this doesnt really make any sense.
> 
> I've created a seperate mesos native lib that only contains a minimal set of 
> code needed to run the ExecutorDriver.  This library has very few 
> requirements, making it a much better option for running in docker 
> containers, as well as allowing a thermos_executor.pex built on one linux 
> distro to run on most others.
> 
> The mesos side of this is here: 
> https://github.com/tellapart/mesos/commit/46bedd5db73b535a0e4efe94730500b6da2ec7d8
> and I'll be opening a RB request on the mesos side soon.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/BUILD 7ef81d13afa0089d7e8a779e71b53e0fc1848466 
>   src/main/python/apache/aurora/executor/BUILD 
> 0be65fc7c180d15f5a94bef268652ef7b868dcf7 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
> 
> Diff: https://reviews.apache.org/r/40889/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>



Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.

2015-12-07 Thread John Sirois


> On Dec. 7, 2015, 12:29 p.m., Zameer Manji wrote:
> > I tried to commit this on master and applying the patch failed. John, can 
> > you please rebase this?

Should be all good now.


- John


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


On Dec. 7, 2015, 2:55 p.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40786/
> ---
> 
> (Updated Dec. 7, 2015, 2:55 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a bit of a straw man.  While working on
> https://issues.apache.org/jira/browse/AURORA-987 I found the need for
> `javapoet` and this project spun off the side.
> 
> See the `@Forward` README here for more info: 
> https://github.com/perkuno/forward
> 
> Although I think the end result is desirable, this change does add a
> dependency on a personal project.  I fancied up the presentation with a
> custom org, but its still a personal project. I'd be happy enough to
> move the `@Forward` code over to aurora, but it would require a seperate
> gradle module to ensure the annotation processor is compiled ahead of
> the main module that would use it.
> 
> So kick the tires and let me know what you think.
> 
>  build.gradle 
>  |   6 +-
>  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java   
>  | 185 --
>  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
>  |  13 ++-
>  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java   
>  | 309 --
>  
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> |   5 +-
>  5 files changed, 20 insertions(+), 498 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> b8bd9185cacc6b113b64a13a1b670fac202c795e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 2de178302d3c9aa9a7b23a9eb7ecb6e2f3b40819 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 1415f0cacc694aa7cf0d25e836e764a96fbb8ae2 
> 
> Diff: https://reviews.apache.org/r/40786/diff/
> 
> 
> Testing
> ---
> 
> Green locally `./build-support/jenkins/build.sh`.
> 
> An example of the generated code:
> ```java
> @Generated("uno.perk.forward.apt.Processor")
> class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
>   protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
> 
>   MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
> this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
>   }
> 
>   @Override
>   public Response getRoleSummary() throws TException {
> return this.annotatedAuroraAdmin.getRoleSummary();
>   }
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.

2015-12-07 Thread John Sirois

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

(Updated Dec. 7, 2015, 2:55 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Update to forward-1.0.0 on maven central.

 build.gradle | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)


Repository: aurora


Description
---

This is a bit of a straw man.  While working on
https://issues.apache.org/jira/browse/AURORA-987 I found the need for
`javapoet` and this project spun off the side.

See the `@Forward` README here for more info: https://github.com/perkuno/forward

Although I think the end result is desirable, this change does add a
dependency on a personal project.  I fancied up the presentation with a
custom org, but its still a personal project. I'd be happy enough to
move the `@Forward` code over to aurora, but it would require a seperate
gradle module to ensure the annotation processor is compiled ahead of
the main module that would use it.

So kick the tires and let me know what you think.

 build.gradle  
|   6 +-
 src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
| 185 --
 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java  
|  13 ++-
 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java
| 309 --
 src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
|   5 +-
 5 files changed, 20 insertions(+), 498 deletions(-)


Diffs (updated)
-

  build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
b8bd9185cacc6b113b64a13a1b670fac202c795e 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
2de178302d3c9aa9a7b23a9eb7ecb6e2f3b40819 
  src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
1415f0cacc694aa7cf0d25e836e764a96fbb8ae2 

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


Testing
---

Green locally `./build-support/jenkins/build.sh`.

An example of the generated code:
```java
@Generated("uno.perk.forward.apt.Processor")
class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
  protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;

  MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
  }

  @Override
  public Response getRoleSummary() throws TException {
return this.annotatedAuroraAdmin.getRoleSummary();
  }
...
```


Thanks,

John Sirois



Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Maxim Khutornenko

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


Thanks for looking into this! Just a minor comment below.


src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java (line 116)


Since there are no callers other than the reconciler unit test, I'd suggest 
moving this code into the test itself.


- Maxim Khutornenko


On Dec. 7, 2015, 7:30 p.m., Tengfei Mu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41048/
> ---
> 
> (Updated Dec. 7, 2015, 7:30 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1547
> https://issues.apache.org/jira/browse/AURORA-1547
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> set TaskStatus.slave_id field in TaskReconsiler
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
>   
> src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
> c797914ded51cd123dddef188373fe58e190773a 
>   
> src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
>  1308a1c9884e7759e6139787710b367bbf9cd4e9 
> 
> Diff: https://reviews.apache.org/r/41048/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Tengfei Mu
> 
>



Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Aurora ReviewBot

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

Ship it!


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

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

- Aurora ReviewBot


On Dec. 7, 2015, 7:30 p.m., Tengfei Mu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41048/
> ---
> 
> (Updated Dec. 7, 2015, 7:30 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1547
> https://issues.apache.org/jira/browse/AURORA-1547
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> set TaskStatus.slave_id field in TaskReconsiler
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
>   
> src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
> c797914ded51cd123dddef188373fe58e190773a 
>   
> src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
>  1308a1c9884e7759e6139787710b367bbf9cd4e9 
> 
> Diff: https://reviews.apache.org/r/41048/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Tengfei Mu
> 
>



Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.

2015-12-07 Thread Zameer Manji

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


I tried to commit this on master and applying the patch failed. John, can you 
please rebase this?

- Zameer Manji


On Nov. 30, 2015, 7:05 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40786/
> ---
> 
> (Updated Nov. 30, 2015, 7:05 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a bit of a straw man.  While working on
> https://issues.apache.org/jira/browse/AURORA-987 I found the need for
> `javapoet` and this project spun off the side.
> 
> See the `@Forward` README here for more info: 
> https://github.com/perkuno/forward
> 
> Although I think the end result is desirable, this change does add a
> dependency on a personal project.  I fancied up the presentation with a
> custom org, but its still a personal project. I'd be happy enough to
> move the `@Forward` code over to aurora, but it would require a seperate
> gradle module to ensure the annotation processor is compiled ahead of
> the main module that would use it.
> 
> So kick the tires and let me know what you think.
> 
>  build.gradle 
>  |   6 +-
>  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java   
>  | 185 --
>  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
>  |  13 ++-
>  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java   
>  | 309 --
>  
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> |   5 +-
>  5 files changed, 20 insertions(+), 498 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> b8bd9185cacc6b113b64a13a1b670fac202c795e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a6769e62d61b2851db055e98ee254f707a91d208 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 1415f0cacc694aa7cf0d25e836e764a96fbb8ae2 
> 
> Diff: https://reviews.apache.org/r/40786/diff/
> 
> 
> Testing
> ---
> 
> Green locally `./build-support/jenkins/build.sh`.
> 
> An example of the generated code:
> ```java
> @Generated("uno.perk.forward.apt.Processor")
> class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
>   protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
> 
>   MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
> this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
>   }
> 
>   @Override
>   public Response getRoleSummary() throws TException {
> return this.annotatedAuroraAdmin.getRoleSummary();
>   }
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Tengfei Mu

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

(Updated Dec. 7, 2015, 7:30 p.m.)


Review request for Aurora and Maxim Khutornenko.


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


Repository: aurora


Description
---

set TaskStatus.slave_id field in TaskReconsiler


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
  src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
c797914ded51cd123dddef188373fe58e190773a 
  
src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
 1308a1c9884e7759e6139787710b367bbf9cd4e9 

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


Testing
---

./gradlew -Pq build


Thanks,

Tengfei Mu



Re: Review Request 40786: Replace manual Forwarding* with `@Forward`.

2015-12-07 Thread Zameer Manji

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

Ship it!


This seems to be a minor change that we can reverse and it does remove 
boilerplate code.

- Zameer Manji


On Nov. 30, 2015, 7:05 a.m., John Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40786/
> ---
> 
> (Updated Nov. 30, 2015, 7:05 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is a bit of a straw man.  While working on
> https://issues.apache.org/jira/browse/AURORA-987 I found the need for
> `javapoet` and this project spun off the side.
> 
> See the `@Forward` README here for more info: 
> https://github.com/perkuno/forward
> 
> Although I think the end result is desirable, this change does add a
> dependency on a personal project.  I fancied up the presentation with a
> custom org, but its still a personal project. I'd be happy enough to
> move the `@Forward` code over to aurora, but it would require a seperate
> gradle module to ensure the annotation processor is compiled ahead of
> the main module that would use it.
> 
> So kick the tires and let me know what you think.
> 
>  build.gradle 
>  |   6 +-
>  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java   
>  | 185 --
>  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
>  |  13 ++-
>  src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java   
>  | 309 --
>  
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> |   5 +-
>  5 files changed, 20 insertions(+), 498 deletions(-)
> 
> 
> Diffs
> -
> 
>   build.gradle a3ff2b747566a38e5ae07db204d0e75da5d3bfb6 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> b8bd9185cacc6b113b64a13a1b670fac202c795e 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 89dd8aacafaa3a68afb8d4a0f4a7cba14cfef503 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> a6769e62d61b2851db055e98ee254f707a91d208 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 
> 1415f0cacc694aa7cf0d25e836e764a96fbb8ae2 
> 
> Diff: https://reviews.apache.org/r/40786/diff/
> 
> 
> Testing
> ---
> 
> Green locally `./build-support/jenkins/build.sh`.
> 
> An example of the generated code:
> ```java
> @Generated("uno.perk.forward.apt.Processor")
> class MockDecoratedThriftForwarder implements AnnotatedAuroraAdmin {
>   protected final AnnotatedAuroraAdmin annotatedAuroraAdmin;
> 
>   MockDecoratedThriftForwarder(AnnotatedAuroraAdmin annotatedAuroraAdmin) {
> this.annotatedAuroraAdmin = Objects.requireNonNull(annotatedAuroraAdmin);
>   }
> 
>   @Override
>   public Response getRoleSummary() throws TException {
> return this.annotatedAuroraAdmin.getRoleSummary();
>   }
> ...
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>



Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Tengfei Mu

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

(Updated Dec. 7, 2015, 7:28 p.m.)


Review request for Aurora and Maxim Khutornenko.


Repository: aurora


Description
---

set TaskStatus.slave_id field in TaskReconsiler


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
  src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
c797914ded51cd123dddef188373fe58e190773a 
  
src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
 1308a1c9884e7759e6139787710b367bbf9cd4e9 

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


Testing
---

./gradlew -Pq build


Thanks,

Tengfei Mu



Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Tengfei Mu

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

(Updated Dec. 7, 2015, 7:22 p.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
---

Added testing done section. Add Maxim as reviewer.


Repository: aurora


Description
---

set TaskStatus.slave_id field in TaskReconsiler


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
  src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
c797914ded51cd123dddef188373fe58e190773a 
  
src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
 1308a1c9884e7759e6139787710b367bbf9cd4e9 

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


Testing (updated)
---

./gradlew -Pq build


Thanks,

Tengfei Mu



Re: Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Aurora ReviewBot

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


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

Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:commons:generateThriftResources
:commons:processResources
:commons:classes
:commons:jar
:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor

:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java:155:
 error: Line is longer than 100 characters (found 101).
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 2 mins 6.427 secs


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

- Aurora ReviewBot


On Dec. 7, 2015, 7:16 p.m., Tengfei Mu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41048/
> ---
> 
> (Updated Dec. 7, 2015, 7:16 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> set TaskStatus.slave_id field in TaskReconsiler
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
>   
> src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
> c797914ded51cd123dddef188373fe58e190773a 
>   
> src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
>  1308a1c9884e7759e6139787710b367bbf9cd4e9 
> 
> Diff: https://reviews.apache.org/r/41048/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tengfei Mu
> 
>



Review Request 41048: set TaskStatus.slave_id field in TaskReconsiler

2015-12-07 Thread Tengfei Mu

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

Review request for Aurora.


Repository: aurora


Description
---

set TaskStatus.slave_id field in TaskReconsiler


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
2cdb2f21202c09b6308e3cfa75d2255699b4c2e5 
  src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
c797914ded51cd123dddef188373fe58e190773a 
  
src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java
 1308a1c9884e7759e6139787710b367bbf9cd4e9 

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


Testing
---


Thanks,

Tengfei Mu



Re: Review Request 40922: Thermos: Add ability to forward process output to stdout

2015-12-07 Thread Martin Hrabovcin

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

(Updated Dec. 7, 2015, 1:17 p.m.)


Review request for Aurora.


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


Repository: aurora


Description (updated)
---

This patch will provide way to **optionally** forward process output to stdout 
and stderr along with existing file 
destinations. Its main use is to get logs to docker daemon when using Mesos 
Docker containerizer.

**What was changed:**

New command line option **log_to_std** is added to thermos_executor and 
thermos_runner. If thermos_executor gets this option it will pass it down to 
thermos_runner and runner will pass it down to **Process** class. New class 
**Tee** that mimicks behavior of unix tee utility that allows to split process 
output to file and stdout is core of this patch. subprocess.Popen in Process 
class used be opened with stdout/stderr file parameters. Now it will always get 
openend with subprocess.PIPE for stdout/stderr. Output of these pipes is 
constantly copied to Tee until process exits. If log_to_std is provided Tee 
will split output to expected files and stdout/stderr. Otherwise logs will be 
split to files and /dev/null.


Diffs
-

  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0d02dc1 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 14e8b4b 
  src/main/python/apache/thermos/core/process.py fe95cb3 
  src/main/python/apache/thermos/core/runner.py f949f27 
  src/main/python/apache/thermos/runner/thermos_runner.py bd8cf7f 
  src/test/python/apache/thermos/core/test_process.py 5e6ad2f 

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


Testing (updated)
---

Unit test coverage is provided for new functionality.

I did also manual testing with mesos/docker and I made sure that logs are being 
written to expected files and also same output gets to docker daemon.


Thanks,

Martin Hrabovcin