You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/10/03 00:28:14 UTC
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
Re: Review Request 26288: Fixing unhandled error logging in updater
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26288/
-----------------------------------------------------------
(Updated Oct. 7, 2014, 10:26 p.m.)
Review request for Aurora and Mark Chu-Carroll.
Changes
-------
Updating title.
Summary (updated)
-----------------
Fixing unhandled error logging in updater
Bugs: AURORA-786
https://issues.apache.org/jira/browse/AURORA-786
Repository: aurora
Description (updated)
-------
Populating details rather than messageDEPRECATED in the client updater to fix logging issue.
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
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
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
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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
Re: Review Request 26288: Fixing log_response in context.py
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26288/#review55506
-----------------------------------------------------------
Mark, ping.
- Maxim Khutornenko
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
Posted by Maxim Khutornenko <ma...@apache.org>.
> 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
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
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
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
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
>
>