aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Chu-Carroll" <mchucarr...@twopensource.com>
Subject Re: Review Request 25204: Adding "get" job update client APIs.
Date Tue, 02 Sep 2014 19:35:21 GMT

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



src/main/python/apache/aurora/client/api/__init__.py
<https://reviews.apache.org/r/25204/#comment90799>

    Nit - but why are you changing the parameter comment syntax? We don't use the double-dash
anywhere else in the client.



src/main/python/apache/aurora/client/api/__init__.py
<https://reviews.apache.org/r/25204/#comment90801>

    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.



src/main/python/apache/aurora/client/api/__init__.py
<https://reviews.apache.org/r/25204/#comment90802>

    If we're going to the trouble of abstracting away the actual scheduler interface, I think
we shouldn't be returning the raw Response datatype. This should return the updates, not the
structure wrapping the updates in two layers of indirection.
    
    These kinds of methods should check the result code, raise an exception if the API call
failed, and then return a meaningful, simple result. 
    
    (Same comment applies to get_job_update_details)


- Mark Chu-Carroll


On Aug. 29, 2014, 6: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, 6: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