Re: Review Request 24852: Add command output tests for job create, job killall, job kill
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/ --- (Updated Aug. 22, 2014, 11:04 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- rebase to master. Bugs: aurora-645 https://issues.apache.org/jira/browse/aurora-645 Repository: aurora Description --- Add tests to ensure that IO behavior of client commands is correct. Improve IO behavior in the IO client, so that users get more information. Formorely, many commands succeeded silently, when it would be more helpful to actually tell the user that you did something. (This shouldn't affect scripting, because in these commands, the only result that's needed by a scripter is succeess/failure, which is carried by the exit code.) Diffs (updated) - src/main/python/apache/aurora/client/cli/context.py 0108d9134a94facb25356fe46a5680eab2bcf2d3 src/main/python/apache/aurora/client/cli/jobs.py 109ce59e3e8dc1c6f4fb918522718cc47867020d src/test/python/apache/aurora/client/cli/test_create.py b70363a93572a0ff88541154adfed59b57ef9d30 src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d src/test/python/apache/aurora/client/cli/test_restart.py 14a08e873058374a9bfe4ffbff8ae7279123f264 Diff: https://reviews.apache.org/r/24852/diff/ Testing --- Ran test suite. Thanks, Mark Chu-Carroll
Re: Review Request 24852: Add command output tests for job create, job killall, job kill
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/#review50974 --- src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/24852/#comment88857 General question: what happens to a message returned by the response in case of error? Is it shown anywhere? src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/24852/#comment88858 What log is it suggesting to look at? Scheduler log or a client one (i.e. loglens)? src/test/python/apache/aurora/client/cli/test_create.py https://reviews.apache.org/r/24852/#comment88860 This is the existing test, right? Diff shows it in green for some reason... - Maxim Khutornenko On Aug. 19, 2014, 3:22 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/ --- (Updated Aug. 19, 2014, 3:22 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-645 https://issues.apache.org/jira/browse/aurora-645 Repository: aurora Description --- Add tests to ensure that IO behavior of client commands is correct. Improve IO behavior in the IO client, so that users get more information. Formorely, many commands succeeded silently, when it would be more helpful to actually tell the user that you did something. (This shouldn't affect scripting, because in these commands, the only result that's needed by a scripter is succeess/failure, which is carried by the exit code.) Diffs - src/main/python/apache/aurora/client/cli/context.py 0e10589c2e8ab6decedb12122a0d8c7a81285e18 src/main/python/apache/aurora/client/cli/jobs.py 109ce59e3e8dc1c6f4fb918522718cc47867020d src/test/python/apache/aurora/client/cli/test_create.py ca635bd49acec9596db67839451101a59ba4d761 src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d src/test/python/apache/aurora/client/cli/test_restart.py 14a08e873058374a9bfe4ffbff8ae7279123f264 Diff: https://reviews.apache.org/r/24852/diff/ Testing --- Ran test suite. Thanks, Mark Chu-Carroll
Re: Review Request 24852: Add command output tests for job create, job killall, job kill
On Aug. 19, 2014, 11:47 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/jobs.py, lines 119-121 https://reviews.apache.org/r/24852/diff/1/?file=664290#file664290line119 General question: what happens to a message returned by the response in case of error? Is it shown anywhere? Yes. It's shown by the call to context.log_response on the previous line. On Aug. 19, 2014, 11:47 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 493 https://reviews.apache.org/r/24852/diff/1/?file=664290#file664290line493 What log is it suggesting to look at? Scheduler log or a client one (i.e. loglens)? Client, of course. The client never refers to the scheduler log. Depending on the client's logging settings, the log messages could be shown on stdout, stderr, or just in loglens. On Aug. 19, 2014, 11:47 a.m., Maxim Khutornenko wrote: src/test/python/apache/aurora/client/cli/test_create.py, lines 233-234 https://reviews.apache.org/r/24852/diff/1/?file=664291#file664291line233 This is the existing test, right? Diff shows it in green for some reason... No, this is a new test. It's almost identical to an existing test, except that instead of checking API calls, it's checking output. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/#review50974 --- On Aug. 19, 2014, 11:22 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/ --- (Updated Aug. 19, 2014, 11:22 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-645 https://issues.apache.org/jira/browse/aurora-645 Repository: aurora Description --- Add tests to ensure that IO behavior of client commands is correct. Improve IO behavior in the IO client, so that users get more information. Formorely, many commands succeeded silently, when it would be more helpful to actually tell the user that you did something. (This shouldn't affect scripting, because in these commands, the only result that's needed by a scripter is succeess/failure, which is carried by the exit code.) Diffs - src/main/python/apache/aurora/client/cli/context.py 0e10589c2e8ab6decedb12122a0d8c7a81285e18 src/main/python/apache/aurora/client/cli/jobs.py 109ce59e3e8dc1c6f4fb918522718cc47867020d src/test/python/apache/aurora/client/cli/test_create.py ca635bd49acec9596db67839451101a59ba4d761 src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d src/test/python/apache/aurora/client/cli/test_restart.py 14a08e873058374a9bfe4ffbff8ae7279123f264 Diff: https://reviews.apache.org/r/24852/diff/ Testing --- Ran test suite. Thanks, Mark Chu-Carroll
Re: Review Request 24852: Add command output tests for job create, job killall, job kill
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/#review50981 --- Ship it! Ship It! - Maxim Khutornenko On Aug. 19, 2014, 3:22 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/ --- (Updated Aug. 19, 2014, 3:22 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-645 https://issues.apache.org/jira/browse/aurora-645 Repository: aurora Description --- Add tests to ensure that IO behavior of client commands is correct. Improve IO behavior in the IO client, so that users get more information. Formorely, many commands succeeded silently, when it would be more helpful to actually tell the user that you did something. (This shouldn't affect scripting, because in these commands, the only result that's needed by a scripter is succeess/failure, which is carried by the exit code.) Diffs - src/main/python/apache/aurora/client/cli/context.py 0e10589c2e8ab6decedb12122a0d8c7a81285e18 src/main/python/apache/aurora/client/cli/jobs.py 109ce59e3e8dc1c6f4fb918522718cc47867020d src/test/python/apache/aurora/client/cli/test_create.py ca635bd49acec9596db67839451101a59ba4d761 src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d src/test/python/apache/aurora/client/cli/test_restart.py 14a08e873058374a9bfe4ffbff8ae7279123f264 Diff: https://reviews.apache.org/r/24852/diff/ Testing --- Ran test suite. Thanks, Mark Chu-Carroll
Re: Review Request 24852: Add command output tests for job create, job killall, job kill
On Aug. 19, 2014, 11:47 a.m., Maxim Khutornenko wrote: src/test/python/apache/aurora/client/cli/test_create.py, lines 233-234 https://reviews.apache.org/r/24852/diff/1/?file=664291#file664291line233 This is the existing test, right? Diff shows it in green for some reason... Mark Chu-Carroll wrote: No, this is a new test. It's almost identical to an existing test, except that instead of checking API calls, it's checking output. Maxim Khutornenko wrote: Please, drop the TODO in that case. No need in multiplying them. But don't we want to mark this as a spot that needs to be fixed? - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/#review50974 --- On Aug. 19, 2014, 11:22 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/ --- (Updated Aug. 19, 2014, 11:22 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-645 https://issues.apache.org/jira/browse/aurora-645 Repository: aurora Description --- Add tests to ensure that IO behavior of client commands is correct. Improve IO behavior in the IO client, so that users get more information. Formorely, many commands succeeded silently, when it would be more helpful to actually tell the user that you did something. (This shouldn't affect scripting, because in these commands, the only result that's needed by a scripter is succeess/failure, which is carried by the exit code.) Diffs - src/main/python/apache/aurora/client/cli/context.py 0e10589c2e8ab6decedb12122a0d8c7a81285e18 src/main/python/apache/aurora/client/cli/jobs.py 109ce59e3e8dc1c6f4fb918522718cc47867020d src/test/python/apache/aurora/client/cli/test_create.py ca635bd49acec9596db67839451101a59ba4d761 src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d src/test/python/apache/aurora/client/cli/test_restart.py 14a08e873058374a9bfe4ffbff8ae7279123f264 Diff: https://reviews.apache.org/r/24852/diff/ Testing --- Ran test suite. Thanks, Mark Chu-Carroll
Re: Review Request 24852: Add command output tests for job create, job killall, job kill
On Aug. 19, 2014, 3:47 p.m., Maxim Khutornenko wrote: src/test/python/apache/aurora/client/cli/test_create.py, lines 233-234 https://reviews.apache.org/r/24852/diff/1/?file=664291#file664291line233 This is the existing test, right? Diff shows it in green for some reason... Mark Chu-Carroll wrote: No, this is a new test. It's almost identical to an existing test, except that instead of checking API calls, it's checking output. Maxim Khutornenko wrote: Please, drop the TODO in that case. No need in multiplying them. Mark Chu-Carroll wrote: But don't we want to mark this as a spot that needs to be fixed? It's already used in more than one place. We only need one reminder that covers all similar issues. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/#review50974 --- On Aug. 19, 2014, 3:22 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/ --- (Updated Aug. 19, 2014, 3:22 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-645 https://issues.apache.org/jira/browse/aurora-645 Repository: aurora Description --- Add tests to ensure that IO behavior of client commands is correct. Improve IO behavior in the IO client, so that users get more information. Formorely, many commands succeeded silently, when it would be more helpful to actually tell the user that you did something. (This shouldn't affect scripting, because in these commands, the only result that's needed by a scripter is succeess/failure, which is carried by the exit code.) Diffs - src/main/python/apache/aurora/client/cli/context.py 0e10589c2e8ab6decedb12122a0d8c7a81285e18 src/main/python/apache/aurora/client/cli/jobs.py 109ce59e3e8dc1c6f4fb918522718cc47867020d src/test/python/apache/aurora/client/cli/test_create.py ca635bd49acec9596db67839451101a59ba4d761 src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d src/test/python/apache/aurora/client/cli/test_restart.py 14a08e873058374a9bfe4ffbff8ae7279123f264 Diff: https://reviews.apache.org/r/24852/diff/ Testing --- Ran test suite. Thanks, Mark Chu-Carroll
Re: Review Request 24852: Add command output tests for job create, job killall, job kill
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/#review50994 --- Ship it! Ship It! - David McLaughlin On Aug. 19, 2014, 3:22 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24852/ --- (Updated Aug. 19, 2014, 3:22 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-645 https://issues.apache.org/jira/browse/aurora-645 Repository: aurora Description --- Add tests to ensure that IO behavior of client commands is correct. Improve IO behavior in the IO client, so that users get more information. Formorely, many commands succeeded silently, when it would be more helpful to actually tell the user that you did something. (This shouldn't affect scripting, because in these commands, the only result that's needed by a scripter is succeess/failure, which is carried by the exit code.) Diffs - src/main/python/apache/aurora/client/cli/context.py 0e10589c2e8ab6decedb12122a0d8c7a81285e18 src/main/python/apache/aurora/client/cli/jobs.py 109ce59e3e8dc1c6f4fb918522718cc47867020d src/test/python/apache/aurora/client/cli/test_create.py ca635bd49acec9596db67839451101a59ba4d761 src/test/python/apache/aurora/client/cli/test_kill.py ee64908855a4960f44ce96c810e69dd105d2ce5d src/test/python/apache/aurora/client/cli/test_restart.py 14a08e873058374a9bfe4ffbff8ae7279123f264 Diff: https://reviews.apache.org/r/24852/diff/ Testing --- Ran test suite. Thanks, Mark Chu-Carroll
