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). 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 >
