awesome :) On Fri, Apr 15, 2016 at 2:18 PM, Tomaz Muraus <[email protected]> wrote:
> There we go - https://github.com/apache/libcloud/pull/755 :) > > On Fri, Apr 15, 2016 at 2:08 PM, Jay Rolette <[email protected]> wrote: > > > On Fri, Apr 15, 2016 at 5:26 AM, Tomaz Muraus <[email protected]> wrote: > > > > > Thanks for the report. > > > > > > This does seem like an unintentional regressions in one of the recent > > > releases. > > > > > > It would be great if you can open a pull request with your changes and > we > > > can start from there (we also need some tests for this to make sure we > > > don't accidentally break it again in the future). > > > > > > > I'm reluctant to submit a PR for my patch since it's a bit of a hack. I > > think the conn_kwargs.update() bit is probably the wrong thing to be > doing > > here, but I don't have time to go figure out what is relying on that and > > how to modify it to work correctly with _ex_connection_class_kwargs(). > > > > My patch cheats and let's either work for setting the timeout, but isn't > a > > proper fix. > > > > FWIW, here's the changeset that broke things: > > > > > https://git-wip-us.apache.org/repos/asf?p=libcloud.git;a=commitdiff;h=fe81fef807d0f26b0fe94d55aa59b6979d133ae8 > > > > Maybe the Rackspace folks can rework how they integrated their > > retry/backoff changes? > > > > Not sure if it helps, but it is easy to miss that you are supposed to use > > _ex_connection_class_kwargs() to set the timeout. The docs don't really > > help on this and, at least for me, it required a fair bit of digging in > the > > code and walking through code in the debugger to track down. That's where > > my "You are in a maze of twisty little passages, all alike..." comment in > > my mixin example came from :-) > > > > Jay > > > > On Thu, Apr 14, 2016 at 11:41 PM, Jay Rolette <[email protected]> > wrote: > > > > > > > version: apache-libcloud-1.0.0-pre1 > > > > > > > > libcloud/common/base.py > > > > > > > > In earlier versions of Libcloud (0.16 for sure), the only way that I > > > found > > > > to control the connection timeout for the various storage drivers was > > to > > > > override _ex_connection_class_kwargs() in a subclass of the driver. > For > > > > example: > > > > > > > > class ExampleMixin: > > > > def _ex_connection_class_kwargs(self): > > > > ''' > > > > You are in a maze of twisty little passages, all alike... > > > > ''' > > > > # This method is the only way to push a timeout value down > into > > > the > > > > # underlying socket connection used by the storage driver. > > > > # > > > > # Without this timeout, if the network fails for some reason > > > while > > > > # we are trying to talk to the cloud, the socket can hang > > > forever. > > > > kwargs = super()._ex_connection_class_kwargs() > > > > kwargs['timeout'] = 120 > > > > > > > > return kwargs > > > > > > > > After we upgraded to 1.0.0-pre1, our connection timeout stopped > > working. > > > > Looking in BaseDriver.__init__(), you can see why it doesn't work > > > anymore. > > > > > > > > *Previously (0.16):* > > > > > > > > class BaseDriver(object): > > > > """ > > > > Base driver class from which other classes can inherit from. > > > > """ > > > > > > > > connectionCls = ConnectionKey > > > > > > > > def __init__(self, key, secret=None, secure=True, host=None, > > > port=None, > > > > api_version=None, region=None, **kwargs): > > > > > > > > # snip code that isn't relevant > > > > > > > > conn_kwargs = self._ex_connection_class_kwargs() > > > > self.connection = self.connectionCls(*args, **conn_kwargs) > > > > ... > > > > > > > > *Now (1.0.0-pre1):* > > > > > > > > class BaseDriver(object): > > > > """ > > > > Base driver class from which other classes can inherit from. > > > > """ > > > > > > > > connectionCls = ConnectionKey > > > > > > > > def __init__(self, key, secret=None, secure=True, host=None, > > > port=None, > > > > api_version=None, region=None, **kwargs): > > > > > > > > # snip > > > > > > > > conn_kwargs = self._ex_connection_class_kwargs() > > > > conn_kwargs.update({'timeout': kwargs.pop('timeout', None), > > > > 'retry_delay': kwargs.pop('retry_delay', > > > None), > > > > 'backoff': kwargs.pop('backoff', None), > > > > 'proxy_url': kwargs.pop('proxy_url', > > None)}) > > > > self.connection = self.connectionCls(*args, **conn_kwargs) > > > > ... > > > > > > > > Not sure if this is just a bug or a change in the design, but it > breaks > > > > existing code. > > > > > > > > I did a minimal patch to my repo to fix the timeout stomp, but > > presumably > > > > it's wrong to have the other parameters like this as well. Here's my > > > patch: > > > > > > > > diff -r 5d23cd267cdc libcloud/common/base.py > > > > --- a/libcloud/common/base.py Mon Apr 11 22:08:14 2016 +0000 > > > > +++ b/libcloud/common/base.py Thu Apr 14 21:03:16 2016 +0000 > > > > @@ -1155,8 +1155,12 @@ > > > > self.verify_ssl_cert = kwargs.get('verify_ssl_cert', None) > > > > > > > > conn_kwargs = self._ex_connection_class_kwargs() > > > > - conn_kwargs.update({'timeout': kwargs.pop('timeout', None), > > > > - 'retry_delay': kwargs.pop('retry_delay', > > > > None), > > > > + # iio: Libcloud was stomping on the connection timeout set > in > > > > + # _ex_connection_class_kwargs() > > > > + timeout = kwargs.pop('timeout', None) > > > > + if conn_kwargs.get('timeout', None) is None: > > > > + conn_kwargs['timeout'] = timeout > > > > + conn_kwargs.update({'retry_delay': kwargs.pop('retry_delay', > > > > None), > > > > 'backoff': kwargs.pop('backoff', None), > > > > 'proxy_url': kwargs.pop('proxy_url', > > None)}) > > > > self.connection = self.connectionCls(*args, **conn_kwargs) > > > > > > > > Thanks, > > > > Jay > > > > > > > > > >
