aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 22023: Modify clientv2 to always log messages from the server
Date Wed, 04 Jun 2014 20:05:01 GMT


> On June 4, 2014, 7:15 p.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/cli/context.py, line 127
> > <https://reviews.apache.org/r/22023/diff/2/?file=598967#file598967line127>
> >
> >     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.
> >

I doubt copy/pasting this pattern as it stands now is a better alternative though. Given that
this is a standard log/check/raise sequence I don't see much loss in readability either.


- Maxim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22023/#review44744
-----------------------------------------------------------


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message