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 25204: Adding "get" job update client APIs.
Date Wed, 03 Sep 2014 22:24:23 GMT


> On Sept. 2, 2014, 7:35 p.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/api/__init__.py, line 232
> > <https://reviews.apache.org/r/25204/diff/1/?file=672539#file672539line232>
> >
> >     I think this would be clearer inlined. Right now, it's pretty much an alternate
name for the JobUpdateQuery constructor, but with different parameter names. It makes the
code harder to follow, not easier.
> 
> Maxim Khutornenko wrote:
>     The reason for this is mostly consistency (see build_query above) but I personally
prefer this approach as it decouples query building from the thrift API call.
> 
> Mark Chu-Carroll wrote:
>     In this case, I disagree pretty strongly - but if you must have the separate method,
at least make the argument names match.

Feels like your disagreement is stronger than my preference in this case :). Changed.


- Maxim


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


On Aug. 29, 2014, 10:28 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25204/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 10:28 p.m.)
> 
> 
> Review request for Aurora and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-615
>     https://issues.apache.org/jira/browse/AURORA-615
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding "get" job update client APIs.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/api/__init__.py 90462bf5920786ea1316c75a99c8382cf8c803a1

>   src/test/python/apache/aurora/client/api/test_api.py b47b6db31842fffba797c7f616b5f4deb8d04a86

> 
> Diff: https://reviews.apache.org/r/25204/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python/apache/aurora/client/api:api -s
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


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