> On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote:
> > src/main/python/apache/aurora/executor/gc_executor.py, line 73
> > <https://reviews.apache.org/r/30749/diff/9/?file=859047#file859047line73>
> >
> >     Some documentation would go a long way here. I assume this is the way 
> > to go from `task_id` <-> `path_of_the_checkpoint_stream_on_disk` ?

Added some commentary.


> On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote:
> > src/main/python/apache/aurora/executor/gc_executor.py, line 129
> > <https://reviews.apache.org/r/30749/diff/9/?file=859047#file859047line129>
> >
> >     s/PathDetector/path_detector, I think

whoops, thanks.


> On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote:
> > src/main/python/apache/aurora/executor/gc_executor.py, line 135
> > <https://reviews.apache.org/r/30749/diff/9/?file=859047#file859047line135>
> >
> >     ```
> >     :param task: an instance of a task to retrieve checkpoint path
> >     :type task: RootedTask instance
> >     ```

Does it make sense to pydoc a private method?  i'll add it but i also don't 
want to necessarily set a precedent.


> On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote:
> > src/main/python/apache/thermos/monitoring/garbage.py, line 31
> > <https://reviews.apache.org/r/30749/diff/9/?file=859049#file859049line31>
> >
> >     Mind adding a docstring here and for the `__init__` ?

done


> On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote:
> > src/main/python/apache/thermos/monitoring/garbage.py, line 173
> > <https://reviews.apache.org/r/30749/diff/9/?file=859049#file859049line173>
> >
> >     Everytime I see a namedtuple I cringe a bit (since I really only see 
> > them as a poor replacement for spec'd mocks) but if you don't mind adding 
> > docs here that'd be ~alright.

This code is on the chopping block, so I wouldn't lament the general practices 
here too much.


- Brian


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


On Feb. 9, 2015, 11:16 p.m., Brian Wickman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30749/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2015, 11:16 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1025
>     https://issues.apache.org/jira/browse/AURORA-1025
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This makes the GC executor detect checkpoint roots via the PathDetector 
> interface.  This paves the way to detecting checkpoint roots both from 
> /var/run/thermos and from /var/lib/mesos/**
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/executor/bin/BUILD 
> 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 
>   src/main/python/apache/aurora/executor/bin/gc_executor_main.py 
> b903bcb3630a8a8d50a2008bfae532b2eb947356 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 
> 0752d50015b2ff936f079c4a9f2777172dc00a93 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 43b415bc6c5177be24ec036cc32ae7cbd87fc70f 
>   src/main/python/apache/thermos/bin/thermos.py 
> 161bbdbc4de95c82e2b596e77b0eac7b920eae66 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f 
>   src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py 
> PRE-CREATION 
>   
> src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py
>  PRE-CREATION 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> b1bbc89a822302d8ea12324eb767631326639ebb 
> 
> Diff: https://reviews.apache.org/r/30749/diff/
> 
> 
> Testing
> -------
> 
> ./pants test.pytest --no-fast src/main/python::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Reply via email to