Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Brian Wickman]

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

Review request for Aurora, Joshua Cohen and Stephan Erb.


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


Repository: aurora


Description
---

Add basic test coverage to the TaskGarbageCollector.


Diffs
-

  src/main/python/apache/thermos/monitoring/garbage.py 
53bb0345f963ae168206f34af1d82c67427f41fe 
  src/test/python/apache/thermos/monitoring/BUILD 
9c267775080e0bde08ff4e9b01862c6bee187634 
  src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 

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


Testing
---

./pants test src/test/python/apache/thermos/monitoring:test_garbage


Thanks,

Brian Wickman

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Aurora ReviewBot]

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

Ship it!


Master (36af017) 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 Feb. 26, 2015, 7:20 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 7:20 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Joshua Cohen]

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

Ship it!



src/main/python/apache/thermos/monitoring/garbage.py


safe_mtime?



src/main/python/apache/thermos/monitoring/garbage.py


What's the purpose of these comments?


- Joshua Cohen


On Feb. 26, 2015, 7:20 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 7:20 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Brian Wickman]

> On Feb. 26, 2015, 7:46 p.m., Joshua Cohen wrote:
> > src/main/python/apache/thermos/monitoring/garbage.py, line 83
> > 
> >
> > What's the purpose of these comments?

ugh, keep forgetting to delete these comments


> On Feb. 26, 2015, 7:46 p.m., Joshua Cohen wrote:
> > src/main/python/apache/thermos/monitoring/garbage.py, lines 44-47
> > 
> >
> > safe_mtime?

derp yeah


- Brian


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


On Feb. 26, 2015, 7:20 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 7:20 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Brian Wickman]

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

(Updated Feb. 26, 2015, 8 p.m.)


Review request for Aurora, Joshua Cohen and Stephan Erb.


Changes
---

Use safe_mtime, remove derpy comments.


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


Repository: aurora


Description
---

Add basic test coverage to the TaskGarbageCollector.


Diffs (updated)
-

  src/main/python/apache/thermos/monitoring/garbage.py 
53bb0345f963ae168206f34af1d82c67427f41fe 
  src/test/python/apache/thermos/monitoring/BUILD 
9c267775080e0bde08ff4e9b01862c6bee187634 
  src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 

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


Testing
---

./pants test src/test/python/apache/thermos/monitoring:test_garbage


Thanks,

Brian Wickman

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Aurora ReviewBot]

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

Ship it!


Master (36af017) 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 Feb. 26, 2015, 8 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 8 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Joshua Cohen]

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

Ship it!


Ship It!

- Joshua Cohen


On Feb. 26, 2015, 8 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 8 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Stephan Erb]

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



src/main/python/apache/thermos/monitoring/garbage.py


You have secured all `self._state` accesses with a check that the latter is 
defined, all except this one.


- Stephan Erb


On Feb. 26, 2015, 9 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 9 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Stephan Erb]

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

Ship it!


Ship It!

- Stephan Erb


On Feb. 26, 2015, 9 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 9 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Joe Smith]

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



src/main/python/apache/thermos/monitoring/garbage.py


Optional, but you'd be doing FutureOncallJoe™ a favor by bootstrapping a 
logger and printing out the rm-s here (maybe in the lower methods instead?0



src/test/python/apache/thermos/monitoring/test_garbage.py


If these are going to run in CI, we shouldn't add 'real' disk 
unfortunately. This could roll up into an 'integration' test, but I'm not a fan 
of adding a disk dependency to the tests


- Joe Smith


On Feb. 26, 2015, noon, Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, noon)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Brian Wickman]

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



src/main/python/apache/thermos/monitoring/garbage.py


Thanks for the catch.


- Brian Wickman


On Feb. 26, 2015, 8 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31491/
> ---
> 
> (Updated Feb. 26, 2015, 8 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1155
> https://issues.apache.org/jira/browse/AURORA-1155
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add basic test coverage to the TaskGarbageCollector.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/monitoring/garbage.py 
> 53bb0345f963ae168206f34af1d82c67427f41fe 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 9c267775080e0bde08ff4e9b01862c6bee187634 
>   src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31491/diff/
> 
> 
> Testing
> ---
> 
> ./pants test src/test/python/apache/thermos/monitoring:test_garbage
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>

Re: Review Request 31491: Add basic test coverage to the TaskGarbageCollector.

[Brian Wickman]

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

(Updated Feb. 26, 2015, 10:46 p.m.)


Review request for Aurora, Joshua Cohen and Stephan Erb.


Changes
---

Fix one missing self._state check.


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


Repository: aurora


Description
---

Add basic test coverage to the TaskGarbageCollector.


Diffs (updated)
-

  src/main/python/apache/thermos/monitoring/garbage.py 
53bb0345f963ae168206f34af1d82c67427f41fe 
  src/test/python/apache/thermos/monitoring/BUILD 
9c267775080e0bde08ff4e9b01862c6bee187634 
  src/test/python/apache/thermos/monitoring/test_garbage.py PRE-CREATION 

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


Testing
---

./pants test src/test/python/apache/thermos/monitoring:test_garbage


Thanks,

Brian Wickman