Re: Review Request 27182: Add a test for the thermos resource module

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27182/#review71520 --- src/test/python/apache/thermos/monitoring/test_resource.py

Review Request 30768: Reject None values for TaskPath

2015-02-08 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-08 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review71567 --- pants.ini https://reviews.apache.org/r/30749/#comment117327

Re: Review Request 27182: Add a test for the thermos resource module

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27182/#review71523 --- Tried to merge but got a conflict. Could you re-merge this with

Review Request 30752: Fix executor builds.

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30752/ --- Review request for Aurora. Repository: aurora Description --- The

Re: Review Request 27182: Add a test for the thermos resource module

2015-02-08 Thread Joe Smith
On Feb. 6, 2015, 3:11 p.m., Brian Wickman wrote: In practice I've found that this helps catch issues sooner than later, as we'll see missing dependencies from the main target (which I've done before). Happy to move away if you disagree though (I know it's a tradeoff) - Joe

Re: Review Request 27182: Add a test for the thermos resource module

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27182/#review71522 --- Ship it! Ship It! - Brian Wickman On Feb. 6, 2015, 10 p.m., Joe

Review Request 30749: Port GC executor to PathDetector interface

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review71538 --- Master (11a65d2) is red with this patch.

Re: Review Request 27182: Add a test for the thermos resource module

2015-02-08 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27182/ --- (Updated Feb. 6, 2015, 3:55 p.m.) Review request for Aurora and Brian Wickman.

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 7, 2015, 2:27 a.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 30647: Instrument the HealthChecker to export stats.

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 6, 2015, 11:13 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review71545 --- Ship it! Master (11a65d2) is green with this patch.

Re: Review Request 30768: Reject None values for TaskPath

2015-02-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71568 --- Ship it! Master (11a65d2) is green with this patch.

Re: Review Request 30752: Fix executor builds.

2015-02-08 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30752/#review71527 --- Ship it! Ship It! - Joe Smith On Feb. 6, 2015, 3:52 p.m., Brian

Re: Review Request 27182: Add a test for the thermos resource module

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27182/#review71510 --- Out of curiosity, why this test now? I sort of see this code on

Re: Review Request 30741: Add an interface and implementations of PathDetector.

2015-02-08 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30741/#review71505 --- Ship it! These are all nits, looks good to me.

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 6, 2015, 11:41 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 30695: Implements log rotation in the Thermos runner.

2015-02-08 Thread George Sirois
On Feb. 6, 2015, 6:52 p.m., Brian Wickman wrote: This is super rad. Thanks for taking this on. Before I do a deeper dive, what do you think about making the logrotate policy be specified by the user instead of the framework owner, with a sensible default? For example, if this is

Re: Review Request 30695: Implements log rotation in the Thermos runner.

2015-02-08 Thread George Sirois
On Feb. 6, 2015, 6:52 p.m., Brian Wickman wrote: This is super rad. Thanks for taking this on. Before I do a deeper dive, what do you think about making the logrotate policy be specified by the user instead of the framework owner, with a sensible default? For example, if this is

Re: Review Request 30741: Add an interface and implementations of PathDetector.

2015-02-08 Thread Brian Wickman
On Feb. 6, 2015, 9:44 p.m., Joshua Cohen wrote: src/main/python/apache/thermos/monitoring/detector.py, line 45 https://reviews.apache.org/r/30741/diff/2/?file=852426#file852426line45 Just for my own education, what's the difference between returning `self._paths` and

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-08 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 7, 2015, 2:43 a.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 30741: Add an interface and implementations of PathDetector.

2015-02-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30741/#review71518 --- Ship it! Master (da296a3) is green with this patch.

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review71544 --- Ship it! Master (11a65d2) is green with this patch.

Re: Review Request 30741: Add an interface and implementations of PathDetector.

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30741/ --- (Updated Feb. 6, 2015, 10:40 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 30749: Port GC executor to PathDetector interface

2015-02-08 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 7, 2015, 12:46 a.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 27182: Add a test for the thermos resource module

2015-02-08 Thread Joe Smith
On Feb. 6, 2015, 1:55 p.m., Brian Wickman wrote: Out of curiosity, why this test now? I sort of see this code on its deathbed as soon Mesos will have disk enforcement built in. This test is from October (and just revived). Until we remove this code, it should have test coverage. - Joe

Re: Review Request 30695: Implements log rotation in the Thermos runner.

2015-02-08 Thread Brian Wickman
On Feb. 6, 2015, 6:52 p.m., Brian Wickman wrote: This is super rad. Thanks for taking this on. Before I do a deeper dive, what do you think about making the logrotate policy be specified by the user instead of the framework owner, with a sensible default? For example, if this is

Re: Review Request 30768: Reject None values for TaskPath

2015-02-08 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71577 --- I would love to see something in the top-level integration test

Re: Review Request 30768: Reject None values for TaskPath

2015-02-08 Thread Bill Farner
On Feb. 8, 2015, 5:28 p.m., Bill Farner wrote: I would love to see something in the top-level integration test that would have caught this. Lacking anything like selenium, we could take on some coupling to low-level details and construct/curl the observer URL: # Find the task

Re: Review Request 30768: Reject None values for TaskPath

2015-02-08 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 1:34 p.m.) Review request for Aurora, Bill Farner and

Re: Review Request 30768: Reject None values for TaskPath

2015-02-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71587 --- Ship it! Master (11a65d2) is green with this patch.

Re: Review Request 30768: Reject None values for TaskPath

2015-02-08 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71589 --- Ship it! LGTM, just a style consistency nit. Thanks for fixing

Re: Review Request 30710: add mesos role feature

2015-02-08 Thread lozh...@ebay.com zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/ --- (Updated Feb. 9, 2015, 4:01 a.m.) Review request for Aurora and Bill Farner.