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 24702: Implementing client job lock and start update APIs.
Date Thu, 14 Aug 2014 17:09:57 GMT

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



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

    Can you add a more specific error type - maybe InvalidConfigError? That way, in the client,
I can generate better error messages.



src/main/python/apache/aurora/client/api/updater_util.py
<https://reviews.apache.org/r/24702/#comment88462>

    This is a bit magical for me. It's relying on subtleties of methods that a typical python
programmer will need to look up, which means that it's going to be really easy for it to be
wrong in a hard-to-recognize way. 
    
    Can you move it up, so that it's a classmethod, and then add a couple of tests? Tests
will both check that it works, and provide demonstrations for readers to see how it works.
(I know you've already got some tests of this, but they're indirect, htrough the surrounding
method; for clarity, it would be better to be able to test get_ranges directly.)


- Mark Chu-Carroll


On Aug. 14, 2014, 12:08 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24702/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2014, 12:08 p.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Bill Farner.
> 
> 
> Bugs: AURORA-615
>     https://issues.apache.org/jira/browse/AURORA-615
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Implementing client job lock and start update APIs.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/api/__init__.py 371137e469fd65935f4048eaafdcf9b702e8c947

>   src/main/python/apache/aurora/client/api/updater_util.py cbb93e4d64a334dba9d091b68cf6d3594930fa31

>   src/test/python/apache/aurora/client/api/BUILD db5c22375378566f58ade5d6a0b7fcce75ad8394

>   src/test/python/apache/aurora/client/api/test_api.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24702/diff/
> 
> 
> Testing
> -------
> 
> ./pants src/test/python/apache/aurora/client/api:api
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


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