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

2015-12-17 Thread George Sirois 

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

(Updated Dec. 17, 2015, 5:22 p.m.)


Review request for Aurora, Bill Farner and Brian Wickman.


Changes
---

@wfarner this change addresses all of your comments + changes the flags be a 
default behavior that can be overridden by process-level configuration.


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


Repository: aurora


Description
---

Implements log rotation in the Thermos runner.


Diffs (updated)
-

  docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
  docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
b3e8bf1e924999306e0b8a1314273b22c51028e7 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
14e8b4bd539d2c295582d93fa01b5613345c1758 
  src/main/python/apache/thermos/config/schema_base.py 
f9143cc1b83143d6147f59d90c79435d055d0518 
  src/main/python/apache/thermos/core/process.py 
fe95cb3be01b47616596bd78cb9a919b2e8bd978 
  src/main/python/apache/thermos/core/runner.py 
f949f279a071c6464b026749f51afc776102f2aa 
  src/main/python/apache/thermos/runner/thermos_runner.py 
bd8cf7f4cda54b6be72dad64f9446eedeb132211 
  src/test/python/apache/thermos/core/test_process.py 
5e6ad2fca616b840299bd9ca1614c82c5c39e992 

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


Testing
---

./pants test src/test/python/apache/thermos/core:all


Thanks,

George Sirois

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

2015-12-17 Thread Aurora ReviewBot 

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


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

virtualenv-12.1.1/virtualenv_support/__init__.py
virtualenv-12.1.1/virtualenv_support/pip-6.1.1-py2.py3-none-any.whl
virtualenv-12.1.1/virtualenv_support/setuptools-15.0-py2.py3-none-any.whl
+ touch virtualenv-12.1.1/BOOTSTRAPPED
+ popd
~/jenkins-slave/workspace/AuroraBot
+ exec /usr/bin/python2.7 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/virtualenv-12.1.1/virtualenv.py
 /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv
New python executable in 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python2.7
Also creating executable in 
/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/isort.venv/bin/python
Installing setuptools, pip...done.
You are using pip version 6.1.1, however version 7.1.2 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting isort==4.0.0
  Using cached isort-4.0.0-py2.py3-none-any.whl
Installing collected packages: isort
Successfully installed isort-4.0.0
ERROR: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/config/schema_base.py
 Imports are incorrectly sorted.
--- 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/config/schema_base.py:before
 2015-12-17 19:35:33.439546
+++ 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/python/apache/thermos/config/schema_base.py:after
  2015-12-17 19:40:34.485465
@@ -14,7 +14,19 @@
 
 # checkstyle: noqa
 
-from pystachio import Boolean, Default, Empty, Enum, Float, Integer, List, 
Map, Required, String, Struct
+from pystachio import (
+Boolean,
+Default,
+Empty,
+Enum,
+Float,
+Integer,
+List,
+Map,
+Required,
+String,
+Struct
+)
 
 # Define constants for resources
 BYTES = 1


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

- Aurora ReviewBot


On Dec. 17, 2015, 5:22 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Dec. 17, 2015, 5:22 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-17 Thread George Sirois 

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

(Updated Dec. 17, 2015, 9:45 p.m.)


Review request for Aurora, Bill Farner and Brian Wickman.


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


Repository: aurora


Description
---

Implements log rotation in the Thermos runner.


Diffs (updated)
-

  NEWS 83a1213e2ea4ab6b3706cd70c015ba9f16735520 
  docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
  docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
b3e8bf1e924999306e0b8a1314273b22c51028e7 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
14e8b4bd539d2c295582d93fa01b5613345c1758 
  src/main/python/apache/thermos/config/schema_base.py 
f9143cc1b83143d6147f59d90c79435d055d0518 
  src/main/python/apache/thermos/core/process.py 
fe95cb3be01b47616596bd78cb9a919b2e8bd978 
  src/main/python/apache/thermos/core/runner.py 
f949f279a071c6464b026749f51afc776102f2aa 
  src/main/python/apache/thermos/runner/thermos_runner.py 
bd8cf7f4cda54b6be72dad64f9446eedeb132211 
  src/test/python/apache/thermos/core/test_process.py 
5e6ad2fca616b840299bd9ca1614c82c5c39e992 

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


Testing
---

./pants test src/test/python/apache/thermos/core:all


Thanks,

George Sirois

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

2015-12-17 Thread Bill Farner 

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

Ship it!


Ship It!

- Bill Farner


On Dec. 17, 2015, 12:38 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Dec. 17, 2015, 12:38 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/configuration-reference.md ad2701cadd38bb2fdbbe2acc477038986f8ec733 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-16 Thread John Sirois 

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

Ship it!


Master (fb8155d) is green with this patch.
  true

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

- John Sirois


On Nov. 25, 2015, 5 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-16 Thread Aurora ReviewBot 

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

Ship it!


Master (fb8155d) 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 Nov. 25, 2015, 5 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-11 Thread Stephan Erb 

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



src/main/python/apache/thermos/core/runner.py (line 718)


I am not sure if it is a good idea to always enforce the gloabal settings, 
instead of using them as a default value in case a process has no logger 
configuration.

I see no harm in allowing a user to override the global default. Afterall, 
it will be his disk quota / resources that will be affected by disabling the 
rotation.


- Stephan Erb


On Nov. 25, 2015, 6 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 6 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-11 Thread George Sirois 


> On Dec. 11, 2015, 1:20 p.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/runner.py, line 718
> > 
> >
> > I am not sure if it is a good idea to always enforce the gloabal 
> > settings, instead of using them as a default value in case a process has no 
> > logger configuration.
> > 
> > I see no harm in allowing a user to override the global default. 
> > Afterall, it will be his disk quota / resources that will be affected by 
> > disabling the rotation.

No objection to this approach either. It's probably the most flexible policy, 
and, as you said, will only affect the individual job owners if they decide to 
do something different than the default.

Bill, any objections?


- George


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


On Nov. 25, 2015, 5 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-11 Thread Bill Farner 


> On Dec. 11, 2015, 5:20 a.m., Stephan Erb wrote:
> > src/main/python/apache/thermos/core/runner.py, line 718
> > 
> >
> > I am not sure if it is a good idea to always enforce the gloabal 
> > settings, instead of using them as a default value in case a process has no 
> > logger configuration.
> > 
> > I see no harm in allowing a user to override the global default. 
> > Afterall, it will be his disk quota / resources that will be affected by 
> > disabling the rotation.
> 
> George Sirois wrote:
> No objection to this approach either. It's probably the most flexible 
> policy, and, as you said, will only affect the individual job owners if they 
> decide to do something different than the default.
> 
> Bill, any objections?

Fine by me.


- Bill


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


On Nov. 25, 2015, 9 a.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 9 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-10 Thread Bill Farner 

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


LGTM mod some top-level ergonomics.  Happy to ship once we resolve those, 
thanks for reviving this!


docs/deploying-aurora-scheduler.md (line 182)


This has diverged from the actual arg in the code.



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


It would be super helpful to list the acceptable values.



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


What is the unit?  FWIW i'm typically fond when the arg calls out the unit 
to make it unambiguous.  I'd also be in favor of making the unit `_mb` as a 
reasonable unit.


- Bill Farner


On Nov. 25, 2015, 9 a.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 9 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-10 Thread Bill Farner 


> On Dec. 10, 2015, 11:12 a.m., Bill Farner wrote:
> > docs/deploying-aurora-scheduler.md, line 182
> > 
> >
> > This has diverged from the actual arg in the code.
> 
> George Sirois wrote:
> Yeah, see my comment with the last change :).
> 
> I wanted to make sure that everyone was on board with the behavior before 
> I finalized the documentation. As things stand right now, if you provide the 
> flags through the scheduler it becomes the behavior for every process, 
> regardless of what you have set in your job config. This is (selfishly) 
> consistent with the original behavior of the patch, but I just wanted to make 
> sure that everyone was onboard with that since it deviates a bit from the 
> earlier conversation in which the flags would supply _default_ values in the 
> absence of any explicit setting in the job config.
> 
> We could meet in the middle and add an extra flag to toggle that behavior 
> (override vs. default) but I'm not sure if you think that would 
> overcomplicate things.

Ah, sorry for the oversight on my part.  The 'big hammer' concern is a valid 
one.  I'm okay with proceeding as-is (full cluster toggle).


- Bill


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


On Nov. 25, 2015, 9 a.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 9 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-10 Thread George Sirois 


> On Dec. 10, 2015, 7:12 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, line 
> > 110
> > 
> >
> > What is the unit?  FWIW i'm typically fond when the arg calls out the 
> > unit to make it unambiguous.  I'd also be in favor of making the unit `_mb` 
> > as a reasonable unit.

SGTM - I'll update with the unit suffix on the arg + update the help 
documentation.


> On Dec. 10, 2015, 7:12 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, line 
> > 104
> > 
> >
> > It would be super helpful to list the acceptable values.

Sure, I'll add them.


> On Dec. 10, 2015, 7:12 p.m., Bill Farner wrote:
> > docs/deploying-aurora-scheduler.md, line 182
> > 
> >
> > This has diverged from the actual arg in the code.

Yeah, see my comment with the last change :).

I wanted to make sure that everyone was on board with the behavior before I 
finalized the documentation. As things stand right now, if you provide the 
flags through the scheduler it becomes the behavior for every process, 
regardless of what you have set in your job config. This is (selfishly) 
consistent with the original behavior of the patch, but I just wanted to make 
sure that everyone was onboard with that since it deviates a bit from the 
earlier conversation in which the flags would supply _default_ values in the 
absence of any explicit setting in the job config.

We could meet in the middle and add an extra flag to toggle that behavior 
(override vs. default) but I'm not sure if you think that would overcomplicate 
things.


- George


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


On Nov. 25, 2015, 5 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-12-10 Thread Martin Hrabovcin 

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


I've manually tested patch on running mesos/aurora cluster. Mesos was able to 
launch jobs, and logs were correctly rotated. +1

- Martin Hrabovcin


On Nov. 25, 2015, 5 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-11-25 Thread Aurora ReviewBot 

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

Ship it!


Master (8524dbf) 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 Nov. 25, 2015, 5 p.m., George Sirois wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30695/
> ---
> 
> (Updated Nov. 25, 2015, 5 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Brian Wickman.
> 
> 
> Bugs: AURORA-95
> https://issues.apache.org/jira/browse/AURORA-95
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implements log rotation in the Thermos runner.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> b3e8bf1e924999306e0b8a1314273b22c51028e7 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 14e8b4bd539d2c295582d93fa01b5613345c1758 
>   src/main/python/apache/thermos/config/schema_base.py 
> f9143cc1b83143d6147f59d90c79435d055d0518 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/core/runner.py 
> f949f279a071c6464b026749f51afc776102f2aa 
>   src/main/python/apache/thermos/runner/thermos_runner.py 
> bd8cf7f4cda54b6be72dad64f9446eedeb132211 
>   src/test/python/apache/thermos/core/test_process.py 
> 5e6ad2fca616b840299bd9ca1614c82c5c39e992 
> 
> Diff: https://reviews.apache.org/r/30695/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/core:all
> 
> 
> Thanks,
> 
> George Sirois
> 
>

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

2015-11-25 Thread George Sirois 

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

(Updated Nov. 25, 2015, 5 p.m.)


Review request for Aurora, Bill Farner and Brian Wickman.


Changes
---

@wickman this is ready for a first pass review now. I didn't bother updating 
the documentation in this changeset since I'm sure we'll iterate on the exact 
naming of flags, etc.


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


Repository: aurora


Description
---

Implements log rotation in the Thermos runner.


Diffs (updated)
-

  docs/deploying-aurora-scheduler.md 8a1e68e5d54e9b8b66139bfc731563668584fa77 
  src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
b3e8bf1e924999306e0b8a1314273b22c51028e7 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
14e8b4bd539d2c295582d93fa01b5613345c1758 
  src/main/python/apache/thermos/config/schema_base.py 
f9143cc1b83143d6147f59d90c79435d055d0518 
  src/main/python/apache/thermos/core/process.py 
fe95cb3be01b47616596bd78cb9a919b2e8bd978 
  src/main/python/apache/thermos/core/runner.py 
f949f279a071c6464b026749f51afc776102f2aa 
  src/main/python/apache/thermos/runner/thermos_runner.py 
bd8cf7f4cda54b6be72dad64f9446eedeb132211 
  src/test/python/apache/thermos/core/test_process.py 
5e6ad2fca616b840299bd9ca1614c82c5c39e992 

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


Testing
---

./pants test src/test/python/apache/thermos/core:all


Thanks,

George Sirois

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

2015-11-19 Thread Stephan Erb 


> On Feb. 6, 2015, 7: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 configurable on the process 
> > object, you can have different policies per process, e.g.
> > 
> > ```py
> > class RotatePolicy(Struct):
> >   log_size = Default(Integer, 32*MB)
> >   backups = Default(Integer, 10)
> >   copytruncate = Default(Boolean, False)
> >   compress = Default(Boolean, False)
> >   hangup_command = String
> >   ...
> > 
> > # union
> > class Logger(Struct):
> >   standard = Boolean  # standard i/o
> >   devnull = Boolean   # /dev/null redirection
> >   logrotate = RotatePolicy  # use a logrotation policy
> > 
> > DefaultLogger = Logger(standard=True)
> > 
> > class Process(Struct):
> >   cmdline = Required(String)
> >   name= Required(String)
> >   ...
> >   logger  = Default(Logger, DefaultLogger)
> > ```
> > 
> > This also means reduced end-to-end plumbing through all the binaries, class 
> > constructors, etc.  And if you ever need to add new features (e.g. a 
> > compress option), they're fairly well encapsulated within the Logger union.
> 
> George Sirois wrote:
> Awesome, thanks for the feedback.
> 
> I'd be willing to take this on; it would definitely make the plumbing a 
> lot cleaner and provide more flexibility, although the downside is that it's 
> now harder to apply a universal default (besides whatever we arrive at as the 
> Aurora default).
> 
> I'll be able to pick this up next week and can probably have a modified 
> review out by Wednesday evening. What do you think about starting out with a 
> simple configuration (just log_size and backups on RotatePolicy) and 
> iterating from there? 
> 
> I also have one question - what distinction are you making between the 
> "standard" flag on Logger and the existence of a rotation policy?
> 
> Brian Wickman wrote:
> Yeah, all the extra parameters were just for illustration only.  Not 
> asking for any more functionality than what you already have since it already 
> provides tremendous value.
> 
> The idea for 'standard' in Logger is just to be explicit about current 
> behavior (unrestricted logging to stdout/stderr) and use it as the default.
> 
> As for applying a universal default that's not "standard", there are a 
> few ways that you could do this, from environment variables 
> (THERMOS_FORCE_ROTATE? idk) to building an aurora client using a custom entry 
> point that patches Process.TYPEMAP['logger'] to use a different default.  
> Both are kind of sketch but within the realm of sketch found elsewhere in the 
> code.
> 
> George Sirois wrote:
> The 'standard' flag makes sense to me, thanks.
> 
> What do you envision reading the environment variable? The 
> executor/runner? I suppose we could enhance the scheduler so that you can 
> pass it environment variables to set when launching the executor so there 
> wouldn't be a lot of plumbing.
> 
> I guess in general I'm not a huge fan of using the client to enforce 
> basic operational parameters like this (although I guess it's debatable as to 
> whether or not these settings qualify :)). For example, it makes it much more 
> challenging to move to a model where jobs are created/started through native 
> API calls to the scheduler instead of using the client.
> 
> Brian Wickman wrote:
> Sorry, I totally missed this follow-up comment.
> 
> If you want to enforce defaults with the client out of the picture, then 
> probably the best way to do this is to still implement the plumbing as 
> described above but omit Default(Logger, DefaultLogger), letting it be Empty 
> by default.
> 
> Add command line parameters to thermos_runner that allow you to toggle 
> which logger is the default (e.g. --process_logger='rotate' 
> --rotate_log_size=...)
> 
> With this in place, you can create a new TaskRunnerProvider 
> (TellApartThermosTaskRunnerProvider? :-) or add flags to the default one that 
> get plumbed through to the aurora_executor command line.  (e.g. 
> --default_process_logger="rotate")
> 
> This at least allows you to set organization-wide policy and will be 
> future-proof if/when the client goes the way of the dodo in favor of a REST 
> API.
> 
> George Sirois wrote:
> Awesome, thanks. My preference is just for an extra flag to keep things 
> simpler on our end for now.
> 
> Zameer Manji wrote:
> George, are you still working on this?
> 
> George Sirois wrote:
> Yeah, sorry for the delay. I'll have it cleaned up and in review this 
> week.

Friendly ping. We would probably be interested in a feature like that as well.


- Stephan


---
This is an automatically generated e-mail. To