Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 6, 2014, 11:38 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Address Maxim's review. Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs (updated) - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py fe11f38b902ae54a4048ba114055ba30e8abe6c5 src/test/python/apache/aurora/client/cli/test_cron.py 049405a4323fc73102d6ac1dae2230be4325c530 src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/#review44926 --- Ship it! src/main/python/apache/aurora/client/cli/cron.py https://reviews.apache.org/r/22023/#comment79512 extra space src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/22023/#comment79517 revert to previous tabbing? src/main/python/apache/aurora/client/cli/task.py https://reviews.apache.org/r/22023/#comment79518 extra space - Maxim Khutornenko On June 6, 2014, 3:38 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 6, 2014, 3:38 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py fe11f38b902ae54a4048ba114055ba30e8abe6c5 src/test/python/apache/aurora/client/cli/test_cron.py 049405a4323fc73102d6ac1dae2230be4325c530 src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 4, 2014, 9:08 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Removed Bill from the reviewers list, since he's out getting married! Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py 2b8ea26e0362690cef38a1b907642d07aa1df37f src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/#review44740 --- src/main/python/apache/aurora/client/cli/context.py https://reviews.apache.org/r/22023/#comment79250 This pattern repeats making messageDEPRECATION removal harder. How about having something like: check_log_and_raise(self, resp, error) method that would do what check_and_log_response does but raise a custom error? src/main/python/apache/aurora/client/cli/cron.py https://reviews.apache.org/r/22023/#comment79251 A similar pattern but without raising an error. Perhaps something like check_and_log_custom_error(self, resp, context)? - Maxim Khutornenko On June 4, 2014, 1:08 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 4, 2014, 1:08 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py 2b8ea26e0362690cef38a1b907642d07aa1df37f src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
Re: Review Request 22023: Modify clientv2 to always log messages from the server
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/#review44744 --- src/main/python/apache/aurora/client/cli/context.py https://reviews.apache.org/r/22023/#comment79263 I've got a strong preference to not do that. When I'm reading code, I find that things like that are a distraction: I can't see the control flow of the method; to understand how it works, I need to jump around the code. I also don't see much benefit in this case: in order to generate the error messages, you need to create the error anyway, and to do that, you need the message anyway. src/main/python/apache/aurora/client/cli/cron.py https://reviews.apache.org/r/22023/#comment79264 Here it's even harder to refactor as you suggest - this does a conditional return. To abstract this out, we'd need to switch to an exception throw, which would be more complex and less clear. - Mark Chu-Carroll On June 4, 2014, 9:08 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22023/ --- (Updated June 4, 2014, 9:08 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-477 https://issues.apache.org/jira/browse/aurora-477 Repository: aurora Description --- - Always show messages returned by the server. - Update message handling in the client for api changes. - Fix some test problems that were uncovered by the API change. Diffs - src/main/python/apache/aurora/client/cli/context.py d1f1f3f308fb3453e79a3f725a3316d25fa4b0f8 src/main/python/apache/aurora/client/cli/cron.py c30a0a605412229f3e4cddbe8c5ae746f256e30c src/main/python/apache/aurora/client/cli/jobs.py 8020c356aba9321ded20f06707ff3678aef61937 src/main/python/apache/aurora/client/cli/quota.py af07d8386e687e3926fd879320245c1eb1c6c263 src/main/python/apache/aurora/client/cli/task.py 2b8ea26e0362690cef38a1b907642d07aa1df37f src/test/python/apache/aurora/client/cli/test_status.py 4cc3f9d66d8f7d8ad66e09d2bfb0dc1a9f9aaa41 src/test/python/apache/aurora/client/cli/test_update.py a2abc5eb0f11f9bc563f4504c93fcf5b7520d141 src/test/python/apache/aurora/client/cli/util.py dac4928111200136a9987c9622087e8cdca7f2d2 Diff: https://reviews.apache.org/r/22023/diff/ Testing --- Ran all client unit tests, plus v2 version of e2e. Thanks, Mark Chu-Carroll
