----------------------------------------------------------- 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. src/main/python/apache/thermos/monitoring/detector.py <https://reviews.apache.org/r/30741/#comment117255> Just for my own education, what's the difference between returning `self._paths` and `self._paths[:]`? src/test/python/apache/aurora/executor/common/test_path_detector.py <https://reviews.apache.org/r/30741/#comment117256> nit: pull '/var/blah/blah' out to a var and use it in the assert below? Same for 'ckpt' src/test/python/apache/thermos/monitoring/test_detector.py <https://reviews.apache.org/r/30741/#comment117258> same here, extract a var and resuse for the call and the assert? (same for the test below as well) src/test/python/apache/thermos/monitoring/test_detector.py <https://reviews.apache.org/r/30741/#comment117259> pull this out to a separate test? (also, same for the test below, could be 3 separate tests) - Joshua Cohen On Feb. 6, 2015, 7:30 p.m., Brian Wickman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30741/ > ----------------------------------------------------------- > > (Updated Feb. 6, 2015, 7:30 p.m.) > > > Review request for Aurora, Joshua Cohen and Zameer Manji. > > > Bugs: AURORA-1024 > https://issues.apache.org/jira/browse/AURORA-1024 > > > Repository: aurora > > > Description > ------- > > The goal here is that any place that uses a fixed checkpoint root that > potentially needs to interact with multiple checkpoint roots (e.g. thermos > observer, thermos cli, gc executor) will instead use a PathDetector > implementation. The default implementation is just the FixedPathDetector to > which you can pass --checkpoint_root. However in subsequent reviews, we will > 1) plumb PathDetector into the abovementioned components and then 2) wire up > a ChainedPathDetector that has both a FixedPathDetector and MesosPathDetector. > > > Diffs > ----- > > src/main/python/apache/aurora/executor/BUILD > 79037bc5af67e5287afa2270b70992cac42de5a7 > src/main/python/apache/aurora/executor/common/BUILD > e64362e3b7d603f51b0545db414a3b3df414594f > src/main/python/apache/aurora/executor/common/path_detector.py PRE-CREATION > src/main/python/apache/aurora/executor/executor_detector.py > 7b8fc4e1cf0b03f7d425a2f6bd0944b583c96737 > src/main/python/apache/aurora/executor/gc_executor.py > 952d77d8ff525ef069a921a987d056de81fc7476 > src/main/python/apache/thermos/monitoring/BUILD > 33259c854f7b28f772310620f9101fb304f30715 > src/main/python/apache/thermos/monitoring/detector.py > 117aef5b0f11dd33781b314be7c80cb4034cc9f8 > src/test/python/apache/aurora/executor/BUILD > aa3bc3b934ea40ee34241521e3f581b0d8ed0ab6 > src/test/python/apache/aurora/executor/common/BUILD > 9e3a657523fc9b848785fcbb0db2e737d371f08f > src/test/python/apache/aurora/executor/common/test_path_detector.py > PRE-CREATION > src/test/python/apache/aurora/executor/test_executor_detector.py > 85d541838c3f74afc022f4ea672e06b90e7e8142 > src/test/python/apache/thermos/monitoring/BUILD > 33d6bba43aff6d62b2646491f004475c27ed99db > src/test/python/apache/thermos/monitoring/test_detector.py PRE-CREATION > > Diff: https://reviews.apache.org/r/30741/diff/ > > > Testing > ------- > > ./pants test src/test/python/apache/aurora/executor/common:: > src/test/python/apache/thermos/monitoring:: > > One failure due to incompatible merge w/ r/30704 -- will send follow up patch. > > > Thanks, > > Brian Wickman > >
