Re: Review Request 30768: Reject None values for TaskPath
On Feb. 8, 2015, 3:20 p.m., Bill Farner wrote: src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py, line 42 https://reviews.apache.org/r/30768/diff/2/?file=857983#file857983line42 This source now has 3 styles for continued lines: - indent matching the first argument - +2 indent - +4 indent AFAIK +4 indent is the agreed-upon convention. Can you fix the whole file to match that? Done. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71589 --- On Feb. 9, 2015, 10:47 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 9, 2015, 10:47 a.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 989801cfcbd19109ac140b01cd3024d70c78c829 Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71649 --- Ship it! Ship It! - Brian Wickman On Feb. 8, 2015, 9:34 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 9:34 p.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 989801cfcbd19109ac140b01cd3024d70c78c829 Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 9, 2015, 10:47 a.m.) Review request for Aurora, Bill Farner and Brian Wickman. Changes --- Style Fixes. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs (updated) - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 989801cfcbd19109ac140b01cd3024d70c78c829 Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71568 --- Ship it! Master (11a65d2) 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. 8, 2015, 2:06 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 2:06 a.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71577 --- I would love to see something in the top-level integration test that would have caught this. Lacking anything like selenium, we could take on some coupling to low-level details and construct/curl the observer URL: # Find the task ID. task_id=$(aurora_admin query --states=RUNNING -l '%taskId%' devcluster www-data hello) # Get the HTTP response code from the observer. check_url_live http://localhost:1338/task/$task_id Do you think that's reasonable? - Bill Farner On Feb. 8, 2015, 2:06 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 2:06 a.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
On Feb. 8, 2015, 5:28 p.m., Bill Farner wrote: I would love to see something in the top-level integration test that would have caught this. Lacking anything like selenium, we could take on some coupling to low-level details and construct/curl the observer URL: # Find the task ID. task_id=$(aurora_admin query --states=RUNNING -l '%taskId%' devcluster www-data hello) # Get the HTTP response code from the observer. check_url_live http://localhost:1338/task/$task_id Do you think that's reasonable? (to be explicit, i'm referring to `src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`) - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71577 --- On Feb. 8, 2015, 2:06 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 2:06 a.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 1:34 p.m.) Review request for Aurora, Bill Farner and Brian Wickman. Changes --- Improve end to end tests to check for observer failure. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs (updated) - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 989801cfcbd19109ac140b01cd3024d70c78c829 Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71587 --- Ship it! Master (11a65d2) 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. 8, 2015, 9:34 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 9:34 p.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 989801cfcbd19109ac140b01cd3024d70c78c829 Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 30768: Reject None values for TaskPath
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/#review71589 --- Ship it! LGTM, just a style consistency nit. Thanks for fixing this! src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py https://reviews.apache.org/r/30768/#comment117381 This source now has 3 styles for continued lines: - indent matching the first argument - +2 indent - +4 indent AFAIK +4 indent is the agreed-upon convention. Can you fix the whole file to match that? - Bill Farner On Feb. 8, 2015, 9:34 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30768/ --- (Updated Feb. 8, 2015, 9:34 p.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1115 https://issues.apache.org/jira/browse/AURORA-1115 Repository: aurora Description --- This patch modifies TaskPath to reject None values. Diffs - src/main/python/apache/aurora/executor/thermos_task_runner.py 7b346e253677ee9b42c57782f7f67ff63b6a0083 src/main/python/apache/thermos/common/path.py 9e617051f16f4270b3958f48e0cc43706d245eec src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py PRE-CREATION src/test/python/apache/thermos/common/test_pathspec.py 3437b196d33d7c2ff6ba292ff99b6881954e7ecb src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 989801cfcbd19109ac140b01cd3024d70c78c829 Diff: https://reviews.apache.org/r/30768/diff/ Testing --- ./pants test src/test/python/apache/thermos:: ./pants test src/test/python/apache/aurora/executor:: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
