Re: Review Request 26288: Fixing log_response in context.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/#review55650 --- Ship it! Looks good, thanks for changing the approach. Can you update the review title, just so that when we look at reviews, we know what this one actually changed? - Mark Chu-Carroll On Oct. 6, 2014, 7:40 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/ --- (Updated Oct. 6, 2014, 7:40 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-786 https://issues.apache.org/jira/browse/AURORA-786 Repository: aurora Description --- Fixing log_response in context.py Diffs - src/main/python/apache/aurora/client/api/updater.py bf608981c2f2e7960b68c3fbda144277a59a3d40 src/test/python/apache/aurora/client/api/test_updater.py 6905831b23a84320e7f41843efd62b86da366c0b Diff: https://reviews.apache.org/r/26288/diff/ Testing --- ./pants src/tests/python:all Thanks, Maxim Khutornenko
Re: Review Request 26288: Fixing log_response in context.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/#review55522 --- src/main/python/apache/aurora/client/cli/context.py https://reviews.apache.org/r/26288/#comment95901 When I wrote code that did that check, Bill specifically told me to remove it, because anything in the deprecated field would *always* also be in the details list. So unless we're moving backwards, this is the wrong approach. If there's a place where the scheduler is sending things via the deprecated field but not via the details list, then that should be changed. - Mark Chu-Carroll On Oct. 2, 2014, 6:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/ --- (Updated Oct. 2, 2014, 6:28 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-786 https://issues.apache.org/jira/browse/AURORA-786 Repository: aurora Description --- Fixing log_response in context.py Diffs - src/main/python/apache/aurora/client/cli/context.py f639af7de93a069b278dc494b6f92a2f6b10de9c src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8 src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION Diff: https://reviews.apache.org/r/26288/diff/ Testing --- ./pants src/tests/python:all Thanks, Maxim Khutornenko
Re: Review Request 26288: Fixing log_response in context.py
On Oct. 6, 2014, 5:12 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/cli/context.py, line 131 https://reviews.apache.org/r/26288/diff/1/?file=712783#file712783line131 When I wrote code that did that check, Bill specifically told me to remove it, because anything in the deprecated field would *always* also be in the details list. So unless we're moving backwards, this is the wrong approach. If there's a place where the scheduler is sending things via the deprecated field but not via the details list, then that should be changed. You are correct about the scheduler duplicating error messages in both fields. That, however, does not cover the updater.py case where the Response object is generated on the client to fit the API return type convention. The alternative would be fixing the updater.py to populate correct fields but I thought this would be the right place to fix in case there are other places on the client where the Response is simulated. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/#review55522 --- On Oct. 2, 2014, 10:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/ --- (Updated Oct. 2, 2014, 10:28 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-786 https://issues.apache.org/jira/browse/AURORA-786 Repository: aurora Description --- Fixing log_response in context.py Diffs - src/main/python/apache/aurora/client/cli/context.py f639af7de93a069b278dc494b6f92a2f6b10de9c src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8 src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION Diff: https://reviews.apache.org/r/26288/diff/ Testing --- ./pants src/tests/python:all Thanks, Maxim Khutornenko
Re: Review Request 26288: Fixing log_response in context.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/#review55531 --- src/main/python/apache/aurora/client/cli/context.py https://reviews.apache.org/r/26288/#comment95906 If the client-side API is using a deprecated field of this structure, then that should be fixed. We shouldn't be patching this to prevent the need to remove deprecations from the client-side API. - Mark Chu-Carroll On Oct. 2, 2014, 6:28 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/ --- (Updated Oct. 2, 2014, 6:28 p.m.) Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-786 https://issues.apache.org/jira/browse/AURORA-786 Repository: aurora Description --- Fixing log_response in context.py Diffs - src/main/python/apache/aurora/client/cli/context.py f639af7de93a069b278dc494b6f92a2f6b10de9c src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8 src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION Diff: https://reviews.apache.org/r/26288/diff/ Testing --- ./pants src/tests/python:all Thanks, Maxim Khutornenko
Re: Review Request 26288: Fixing log_response in context.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/ --- (Updated Oct. 6, 2014, 11:40 p.m.) Review request for Aurora and Mark Chu-Carroll. Changes --- Moving the fix into the client updater. Bugs: AURORA-786 https://issues.apache.org/jira/browse/AURORA-786 Repository: aurora Description --- Fixing log_response in context.py Diffs (updated) - src/main/python/apache/aurora/client/api/updater.py bf608981c2f2e7960b68c3fbda144277a59a3d40 src/test/python/apache/aurora/client/api/test_updater.py 6905831b23a84320e7f41843efd62b86da366c0b Diff: https://reviews.apache.org/r/26288/diff/ Testing --- ./pants src/tests/python:all Thanks, Maxim Khutornenko
Review Request 26288: Fixing log_response in context.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/ --- Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-786 https://issues.apache.org/jira/browse/AURORA-786 Repository: aurora Description --- Fixing log_response in context.py Diffs - src/main/python/apache/aurora/client/cli/context.py f639af7de93a069b278dc494b6f92a2f6b10de9c src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8 src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION Diff: https://reviews.apache.org/r/26288/diff/ Testing --- ./pants src/tests/python:all Thanks, Maxim Khutornenko
