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