You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by to...@apache.org on 2013/12/07 20:18:27 UTC

git commit: [LIBCLOUD-457] Reset context on the Connection class after the request has finished.

Updated Branches:
  refs/heads/trunk 2eca5c2d4 -> 67910fd51


[LIBCLOUD-457] Reset context on the Connection class after the request has
finished.

Also update affected Abiquo driver to use "context" attribute correctly and
not rely on it being present across multiple requests. Instead of the
"context", use a new attribute called "cache".


Project: http://git-wip-us.apache.org/repos/asf/libcloud/repo
Commit: http://git-wip-us.apache.org/repos/asf/libcloud/commit/67910fd5
Tree: http://git-wip-us.apache.org/repos/asf/libcloud/tree/67910fd5
Diff: http://git-wip-us.apache.org/repos/asf/libcloud/diff/67910fd5

Branch: refs/heads/trunk
Commit: 67910fd5189d2f9b3078c16601cef714cb86d486
Parents: 2eca5c2
Author: Tomaz Muraus <to...@apache.org>
Authored: Sat Dec 7 19:07:35 2013 +0100
Committer: Tomaz Muraus <to...@apache.org>
Committed: Sat Dec 7 20:14:18 2013 +0100

----------------------------------------------------------------------
 libcloud/common/abiquo.py          | 10 +++++++
 libcloud/common/base.py            | 21 ++++++++++++--
 libcloud/compute/drivers/abiquo.py | 37 ++++++++++++------------
 libcloud/test/test_connection.py   | 51 +++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/libcloud/blob/67910fd5/libcloud/common/abiquo.py
----------------------------------------------------------------------
diff --git a/libcloud/common/abiquo.py b/libcloud/common/abiquo.py
index 7c367eb..f621b2a 100644
--- a/libcloud/common/abiquo.py
+++ b/libcloud/common/abiquo.py
@@ -173,6 +173,16 @@ class AbiquoConnection(ConnectionUserAndKey, PollingConnection):
 
     responseCls = AbiquoResponse
 
+    def __init__(self, user_id, key, secure=True, host=None, port=None,
+                 url=None, timeout=None):
+        super(AbiquoConnection, self).__init__(user_id=user_id, key=key,
+                                               secure=secure,
+                                               host=host, port=port,
+                                               url=url, timeout=timeout)
+
+        # This attribute stores data cached across multiple request
+        self.cache = {}
+
     def add_default_headers(self, headers):
         """
         Add Basic Authentication header to all the requests.

http://git-wip-us.apache.org/repos/asf/libcloud/blob/67910fd5/libcloud/common/base.py
----------------------------------------------------------------------
diff --git a/libcloud/common/base.py b/libcloud/common/base.py
index 6cccc0a..5ffd182 100644
--- a/libcloud/common/base.py
+++ b/libcloud/common/base.py
@@ -426,8 +426,14 @@ class Connection(object):
             self.timeout = timeout
 
     def set_context(self, context):
+        if not isinstance(context, dict):
+            raise TypeError('context needs to be a dictionary')
+
         self.context = context
 
+    def reset_context(self):
+        self.context = {}
+
     def _tuple_from_url(self, url):
         secure = 1
         port = None
@@ -633,13 +639,22 @@ class Connection(object):
                                         headers=headers)
         except ssl.SSLError:
             e = sys.exc_info()[1]
+            self.reset_context()
             raise ssl.SSLError(str(e))
 
         if raw:
-            response = self.rawResponseCls(connection=self)
+            responseCls = self.rawResponseCls
+            kwargs = {'connection': self}
         else:
-            response = self.responseCls(response=self.connection.getresponse(),
-                                        connection=self)
+            responseCls = self.responseCls
+            kwargs = {'connection': self,
+                      'response': self.connection.getresponse()}
+
+        try:
+            response = responseCls(**kwargs)
+        finally:
+            # Always reset the context after the request has completed
+            self.reset_context()
 
         return response
 

http://git-wip-us.apache.org/repos/asf/libcloud/blob/67910fd5/libcloud/compute/drivers/abiquo.py
----------------------------------------------------------------------
diff --git a/libcloud/compute/drivers/abiquo.py b/libcloud/compute/drivers/abiquo.py
index 7ccec66..b3e2d2e 100644
--- a/libcloud/compute/drivers/abiquo.py
+++ b/libcloud/compute/drivers/abiquo.py
@@ -55,8 +55,7 @@ class AbiquoNodeDriver(NodeDriver):
         """
         Initializes Abiquo Driver
 
-        Initializes the :class:`NodeDriver` object. After that, it generates
-        the context
+        Initializes the :class:`NodeDriver` object and populate the cache.
 
         :param       user_id: identifier of Abiquo user (required)
         :type        user_id: ``str``
@@ -69,7 +68,7 @@ class AbiquoNodeDriver(NodeDriver):
         super(AbiquoNodeDriver, self).__init__(key=user_id, secret=secret,
                                                secure=False, host=None,
                                                port=None, **kwargs)
-        self.ex_set_context()
+        self.ex_populate_cache()
 
     def create_node(self, **kwargs):
         """
@@ -220,9 +219,9 @@ class AbiquoNodeDriver(NodeDriver):
         e_vm = self.connection.request(edit_vm, headers=headers).object
         return self._to_node(e_vm, self)
 
-    def ex_set_context(self):
+    def ex_populate_cache(self):
         """
-        Generates the context
+        Populate the cache.
 
         For each connection, it is good to store some objects that will be
         useful for further requests, such as the 'user' and the 'enterprise'
@@ -230,7 +229,7 @@ class AbiquoNodeDriver(NodeDriver):
 
         Executes the 'login' resource after setting the connection parameters
         and, if the execution is successful, it sets the 'user' object into
-        context. After that, it also requests for the 'enterprise' and
+        cache. After that, it also requests for the 'enterprise' and
         'locations' data.
 
         List of locations should remain the same for a single libcloud
@@ -238,11 +237,11 @@ class AbiquoNodeDriver(NodeDriver):
         refresh the list of locations any time.
         """
         user = self.connection.request('/login').object
-        self.connection.context['user'] = user
-        e_ent = get_href(self.connection.context['user'],
+        self.connection.cache['user'] = user
+        e_ent = get_href(self.connection.cache['user'],
                          'enterprise')
         ent = self.connection.request(e_ent).object
-        self.connection.context['enterprise'] = ent
+        self.connection.cache['enterprise'] = ent
 
         uri_vdcs = '/cloud/virtualdatacenters'
         e_vdcs = self.connection.request(uri_vdcs).object
@@ -256,18 +255,18 @@ class AbiquoNodeDriver(NodeDriver):
             key = get_href(dc, 'edit')
             dc_dict[key] = dc
 
-        # Set the context for the locations
-        self.connection.context['locations'] = {}
+        # Populate locations cache
+        self.connection.cache['locations'] = {}
         for e_vdc in e_vdcs.findall('virtualDatacenter'):
             dc_link = get_href(e_vdc, 'datacenter')
             loc = self._to_location(e_vdc, dc_dict[dc_link], self)
 
-            # Save into context the link to the itself because we will need
+            # Save into cache the link to the itself because we will need
             # it in the future, but we save here to don't extend the class
             # :class:`NodeLocation`.
             # So here we have the dict: :class:`NodeLocation` ->
             # link_datacenter
-            self.connection.context['locations'][loc] = get_href(e_vdc, 'edit')
+            self.connection.cache['locations'][loc] = get_href(e_vdc, 'edit')
 
     def ex_create_group(self, name, location=None):
         """
@@ -294,7 +293,7 @@ class AbiquoNodeDriver(NodeDriver):
         elif not location in self.list_locations():
             raise LibcloudError('Location does not exist')
 
-        link_vdc = self.connection.context['locations'][location]
+        link_vdc = self.connection.cache['locations'][location]
         e_vdc = self.connection.request(link_vdc).object
 
         creation_link = get_href(e_vdc, 'virtualappliances')
@@ -366,7 +365,7 @@ class AbiquoNodeDriver(NodeDriver):
         """
         groups = []
         for vdc in self._get_locations(location):
-            link_vdc = self.connection.context['locations'][vdc]
+            link_vdc = self.connection.cache['locations'][vdc]
             e_vdc = self.connection.request(link_vdc).object
             apps_link = get_href(e_vdc, 'virtualappliances')
             vapps = self.connection.request(apps_link).object
@@ -404,7 +403,7 @@ class AbiquoNodeDriver(NodeDriver):
             # is different from the 'datacenterRepository' element
             for vdc in self._get_locations(location):
                 # Check if the virtual datacenter belongs to this repo
-                link_vdc = self.connection.context['locations'][vdc]
+                link_vdc = self.connection.cache['locations'][vdc]
                 e_vdc = self.connection.request(link_vdc).object
                 dc_link_vdc = get_href(e_vdc, 'datacenter')
                 dc_link_repo = get_href(repo, 'datacenter')
@@ -435,7 +434,7 @@ class AbiquoNodeDriver(NodeDriver):
                  user
         :rtype:  ``list`` of :class:`NodeLocation`
         """
-        return list(self.connection.context['locations'].keys())
+        return list(self.connection.cache['locations'].keys())
 
     def list_nodes(self, location=None):
         """
@@ -621,7 +620,7 @@ class AbiquoNodeDriver(NodeDriver):
         """
         Returns the identifier of the logged user's enterprise.
         """
-        return self.connection.context['enterprise'].findtext('id')
+        return self.connection.cache['enterprise'].findtext('id')
 
     def _define_create_node_location(self, **kwargs):
         """
@@ -649,7 +648,7 @@ class AbiquoNodeDriver(NodeDriver):
         loc = None
         target_loc = None
         for candidate_loc in self._get_locations(location):
-            link_vdc = self.connection.context['locations'][candidate_loc]
+            link_vdc = self.connection.cache['locations'][candidate_loc]
             e_vdc = self.connection.request(link_vdc).object
             # url_location = get_href(e_vdc, 'datacenter')
             for img in self.list_images(candidate_loc):

http://git-wip-us.apache.org/repos/asf/libcloud/blob/67910fd5/libcloud/test/test_connection.py
----------------------------------------------------------------------
diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py
index 0525dd4..c17ff4d 100644
--- a/libcloud/test/test_connection.py
+++ b/libcloud/test/test_connection.py
@@ -15,6 +15,7 @@
 # limitations under the License.
 
 import sys
+import ssl
 
 from mock import Mock, call
 
@@ -131,5 +132,55 @@ class ConnectionClassTestCase(unittest.TestCase):
         args, kwargs = con.pre_connect_hook.call_args
         self.assertTrue('cache-busting' in args[0][len(params2)])
 
+    def test_context_is_reset_after_request_has_finished(self):
+        context = {'foo': 'bar'}
+
+        def responseCls(connection, response):
+            connection.called = True
+            self.assertEqual(connection.context, context)
+
+        con = Connection()
+        con.called = False
+        con.connection = Mock()
+        con.responseCls = responseCls
+
+        con.set_context(context)
+        self.assertEqual(con.context, context)
+
+        con.request('/')
+
+        # Context should have been reset
+        self.assertTrue(con.called)
+        self.assertEqual(con.context, {})
+
+        # Context should also be reset if a method inside request throws
+        con = Connection()
+        con.connection = Mock()
+
+        con.set_context(context)
+        self.assertEqual(con.context, context)
+
+        con.connection.request = Mock(side_effect=ssl.SSLError())
+
+        try:
+            con.request('/')
+        except ssl.SSLError:
+            pass
+
+        self.assertEqual(con.context, {})
+
+        con.connection = Mock()
+        con.set_context(context)
+        self.assertEqual(con.context, context)
+
+        con.responseCls = Mock(side_effect=ValueError())
+
+        try:
+            con.request('/')
+        except ValueError:
+            pass
+
+        self.assertEqual(con.context, {})
+
 if __name__ == '__main__':
     sys.exit(unittest.main())