> On Feb. 26, 2015, 4:25 a.m., Zameer Manji wrote:
> > src/main/python/apache/thermos/bin/thermos.py, line 289
> > <https://reviews.apache.org/r/31451/diff/2/?file=877122#file877122line289>
> >
> >     How is this change related to the observer change?

I was testing the observer by doing local thermos runs, and this command was 
broken, so I fixed it.


> On Feb. 26, 2015, 4:25 a.m., Zameer Manji wrote:
> > src/main/python/apache/thermos/observer/detector.py, line 32
> > <https://reviews.apache.org/r/31451/diff/2/?file=877130#file877130line32>
> >
> >     Can you please confirm if this property is only used in testing?

It is not only used by tests.  The next observer CL has a component called the 
TaskDatabase that uses it.


> On Feb. 26, 2015, 4:25 a.m., Zameer Manji wrote:
> > src/main/python/apache/thermos/observer/detector.py, line 78
> > <https://reviews.apache.org/r/31451/diff/2/?file=877130#file877130line78>
> >
> >     Please put a TODO(wickman) instead of a TBD

Hrm, this code is already janky.  I will make it more concise and send back a 
second diff on this review.


> On Feb. 26, 2015, 4:25 a.m., Zameer Manji wrote:
> > src/main/python/apache/thermos/observer/task_observer.py, line 147
> > <https://reviews.apache.org/r/31451/diff/2/?file=877133#file877133line147>
> >
> >     Why is the detector protected witha lock but none of the callbacks are?
> >     
> >     I think either the detector should have an internal lock to prevent 
> > concurrent refreshes or the lock here should also be used to protect the 
> > callbacks that are passed in.

refreshes only happen in the run() thread.  the callbacks are guaranteed to 
only be invoked by .refresh(), therefore protecting it with the lock is fine.  
the main need for the lock is for the Lockable.sync read methods (which will be 
going away in the next review.)


- Brian


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


On Feb. 26, 2015, 1:50 a.m., Brian Wickman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31451/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2015, 1:50 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Zameer Manji.
> 
> 
> Bugs: AURORA-1026
>     https://issues.apache.org/jira/browse/AURORA-1026
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This creates a new abstraction, the ObserverTaskDetector, which is 
> responsible for managing state transitions for tasks for the observer.  Adds 
> some tests and better debug logging.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/executor/common/resource_manager.py 
> 08e02e41b581f275f070228bb23c4cf2a0489f9a 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> 7a28e3255842e3e13a0866d6ad1bfc4cb64781e1 
>   src/main/python/apache/thermos/bin/thermos.py 
> 0853a9892399824385bee9e72db4c108f46fceda 
>   src/main/python/apache/thermos/common/path.py 
> 846f507e2e097fc04fe0098a7250b40fefcfc6e2 
>   src/main/python/apache/thermos/monitoring/disk.py 
> 175ed3af6515e6107e297d91d4e30cbb3034faf7 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 11423bc1764c8380d8de4ad095c1e2748ebb78f8 
>   src/main/python/apache/thermos/monitoring/resource.py 
> b4cb881c87a09bb90a740f369a7a5fc5d75dbf04 
>   src/main/python/apache/thermos/observer/BUILD 
> ee65f3a46e1d339620e76cadae92c6678fc3510f 
>   src/main/python/apache/thermos/observer/bin/BUILD 
> 15a03f74f204f58856f0843b9db05e83b89d1138 
>   src/main/python/apache/thermos/observer/bin/thermos_observer.py 
> effa8c19f963bf2792497f4a06049214ae30dfa5 
>   src/main/python/apache/thermos/observer/detector.py PRE-CREATION 
>   src/main/python/apache/thermos/observer/http/file_browser.py 
> 87ef9c8a29689c78a5e39a46cc53e4675e36a381 
>   src/main/python/apache/thermos/observer/observed_task.py 
> f33aecbc8f3c0a461ae3dba66fbd4986f544dc04 
>   src/main/python/apache/thermos/observer/task_observer.py 
> cd528dcca3f5a330359cf38005f3a1a0329a4886 
>   src/test/python/apache/thermos/observer/BUILD PRE-CREATION 
>   src/test/python/apache/thermos/observer/test_detector.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31451/diff/
> 
> 
> Testing
> -------
> 
> Manually launched observer and ran some thermos tasks.
> +
> mba=aurora=; ./pants test src/test/python/apache/thermos/observer/::
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Reply via email to