You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@aurora.apache.org by Brian Wickman <wi...@gmail.com> on 2013/12/10 20:42:49 UTC

Re: Review Request 16144: ZookeeperSchedulerClient url property errored when no proxy_url was set, and the client was not connected.

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

Ship it!


seems reasonable to me.

- Brian Wickman


On Dec. 10, 2013, 1:57 a.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16144/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2013, 1:57 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
> 
> 
> Bugs: AURORA-6
>     https://issues.apache.org/jira/browse/AURORA-6
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> commit 69f4b79e4054b85b36490c04fb569e3f752dca6f
> Author: Joshua Cohen <jc...@twitter.com>
> Date:   Mon Dec 9 18:09:22 2013 -0600
> 
>     ZookeeperSchedulerClient url property errored when no proxy_url was set, and the client was not connected.
> 
>  .../twitter/aurora/client/api/scheduler_client.py  |  3 ++
>  .../aurora/client/api/test_scheduler_client.py     | 56 ++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> The original failure is:
> 
>   File "twitter/aurora/client/api/scheduler_client.py", line 138, in url
>     if self._http:
>   AttributeError: 'ZookeeperSchedulerClient' object has no attribute '_http'
> 
> The fix presented here is to implicitly connect if we're not connected though I'm not sure if this is the desired behavior. The alternative would be to gracefully fail if cluster.proxy_url is not set and self._http is None, then leave it up to the caller to decide whether or not to connect. Connecting is generally implicit today though, so I went this way (though connecting as a result of trying to access the scheduler url may be unexpected).
> 
> 
> Diffs
> -----
> 
>   src/main/python/twitter/aurora/client/api/scheduler_client.py ffec6043ecab7215eb6be523b879259976c4b2c8 
>   src/test/python/twitter/aurora/client/api/test_scheduler_client.py fe62cd24f3da755859b1fbbfe9a1ee8f91116606 
> 
> Diff: https://reviews.apache.org/r/16144/diff/
> 
> 
> Testing
> -------
> 
> Added unit test, ./pants src/test/python/twitter/aurora/client:all
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>


Re: Review Request 16144: ZookeeperSchedulerClient url property errored when no proxy_url was set, and the client was not connected.

Posted by Zameer Manji <zm...@gmail.com>.

> On Dec. 10, 2013, 11:42 a.m., Brian Wickman wrote:
> > seems reasonable to me.

Brian, you have to commit the patch since Joshua does not have commit rights. Once this is committed Joshua can close this review. 


- Zameer


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


On Dec. 9, 2013, 5:57 p.m., Joshua Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16144/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2013, 5:57 p.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Brian Wickman.
> 
> 
> Bugs: AURORA-6
>     https://issues.apache.org/jira/browse/AURORA-6
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> commit 69f4b79e4054b85b36490c04fb569e3f752dca6f
> Author: Joshua Cohen <jc...@twitter.com>
> Date:   Mon Dec 9 18:09:22 2013 -0600
> 
>     ZookeeperSchedulerClient url property errored when no proxy_url was set, and the client was not connected.
> 
>  .../twitter/aurora/client/api/scheduler_client.py  |  3 ++
>  .../aurora/client/api/test_scheduler_client.py     | 56 ++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> The original failure is:
> 
>   File "twitter/aurora/client/api/scheduler_client.py", line 138, in url
>     if self._http:
>   AttributeError: 'ZookeeperSchedulerClient' object has no attribute '_http'
> 
> The fix presented here is to implicitly connect if we're not connected though I'm not sure if this is the desired behavior. The alternative would be to gracefully fail if cluster.proxy_url is not set and self._http is None, then leave it up to the caller to decide whether or not to connect. Connecting is generally implicit today though, so I went this way (though connecting as a result of trying to access the scheduler url may be unexpected).
> 
> 
> Diffs
> -----
> 
>   src/main/python/twitter/aurora/client/api/scheduler_client.py ffec6043ecab7215eb6be523b879259976c4b2c8 
>   src/test/python/twitter/aurora/client/api/test_scheduler_client.py fe62cd24f3da755859b1fbbfe9a1ee8f91116606 
> 
> Diff: https://reviews.apache.org/r/16144/diff/
> 
> 
> Testing
> -------
> 
> Added unit test, ./pants src/test/python/twitter/aurora/client:all
> 
> 
> Thanks,
> 
> Joshua Cohen
> 
>