You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@libcloud.apache.org by Jay Rolette <ro...@infinite.io> on 2016/04/14 23:41:18 UTC
[dev] BaseDriver.__init__() stomps over connection timeout
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
Re: [dev] BaseDriver.__init__() stomps over connection timeout
Posted by Tomaz Muraus <to...@apache.org>.
Alright.
In this case it would be great if you can provide a test case / code which
breaks so we use this as a starting point for the fix :)
On Fri, Apr 15, 2016 at 2:08 PM, Jay Rolette <ro...@infinite.io> wrote:
> On Fri, Apr 15, 2016 at 5:26 AM, Tomaz Muraus <to...@apache.org> 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 <ro...@infinite.io> 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
> > >
> >
>
Re: [dev] BaseDriver.__init__() stomps over connection timeout
Posted by Jay Rolette <ro...@infinite.io>.
awesome :)
On Fri, Apr 15, 2016 at 2:18 PM, Tomaz Muraus <to...@apache.org> wrote:
> There we go - https://github.com/apache/libcloud/pull/755 :)
>
> On Fri, Apr 15, 2016 at 2:08 PM, Jay Rolette <ro...@infinite.io> wrote:
>
> > On Fri, Apr 15, 2016 at 5:26 AM, Tomaz Muraus <to...@apache.org> 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 <ro...@infinite.io>
> 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
> > > >
> > >
> >
>
Re: [dev] BaseDriver.__init__() stomps over connection timeout
Posted by Tomaz Muraus <to...@apache.org>.
There we go - https://github.com/apache/libcloud/pull/755 :)
On Fri, Apr 15, 2016 at 2:08 PM, Jay Rolette <ro...@infinite.io> wrote:
> On Fri, Apr 15, 2016 at 5:26 AM, Tomaz Muraus <to...@apache.org> 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 <ro...@infinite.io> 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
> > >
> >
>
Re: [dev] BaseDriver.__init__() stomps over connection timeout
Posted by Jay Rolette <ro...@infinite.io>.
On Fri, Apr 15, 2016 at 5:26 AM, Tomaz Muraus <to...@apache.org> 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 <ro...@infinite.io> 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
> >
>
Re: [dev] BaseDriver.__init__() stomps over connection timeout
Posted by Tomaz Muraus <to...@apache.org>.
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 <ro...@infinite.io> 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
>