[lxc-devel] [pylxd/master] Extraneous exceptions

rockstar on Github lxc-bot at linuxcontainers.org
Tue Jun 28 19:43:15 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 467 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160628/8b6fb75c/attachment.bin>
-------------- next part --------------
From 50aa3095f1c72a80aa91a3a6cd21d529903e7e10 Mon Sep 17 00:00:00 2001
From: Paul Hummer <paul.hummer at canonical.com>
Date: Tue, 28 Jun 2016 13:19:31 -0600
Subject: [PATCH 1/3] Remove unneeded/useless exceptions

---
 pylxd/container.py            | 35 ++++++--------------------
 pylxd/exceptions.py           | 57 ++++---------------------------------------
 pylxd/image.py                | 24 ++++--------------
 pylxd/model.py                |  8 +-----
 pylxd/profile.py              | 16 +++---------
 pylxd/tests/test_container.py | 31 +++++------------------
 pylxd/tests/test_image.py     | 30 +++++------------------
 pylxd/tests/test_profile.py   | 27 +++-----------------
 8 files changed, 38 insertions(+), 190 deletions(-)

diff --git a/pylxd/container.py b/pylxd/container.py
index 4e00007..3f49b32 100644
--- a/pylxd/container.py
+++ b/pylxd/container.py
@@ -18,7 +18,7 @@
 from ws4py.client import WebSocketBaseClient
 from ws4py.manager import WebSocketManager
 
-from pylxd import exceptions, managers, model
+from pylxd import managers, model
 from pylxd.deprecation import deprecated
 from pylxd.operation import Operation
 
@@ -72,26 +72,15 @@ def put(self, filepath, data):
             return response.status_code == 200
 
         def get(self, filepath):
-            try:
-                response = self._client.api.containers[
-                    self._container.name].files.get(
-                    params={'path': filepath})
-            except exceptions.LXDAPIException as e:
-                # LXD 2.0.3+ return 404, not 500,
-                if e.response.status_code in (500, 404):
-                    raise exceptions.NotFound()
-                raise
+            response = self._client.api.containers[
+                self._container.name].files.get(
+                params={'path': filepath})
             return response.content
 
     @classmethod
     def get(cls, client, name):
         """Get a container by name."""
-        try:
-            response = client.api.containers[name].get()
-        except exceptions.LXDAPIException as e:
-            if e.response.status_code == 404:
-                raise exceptions.NotFound()
-            raise
+        response = client.api.containers[name].get()
 
         container = cls(client, **response.json()['metadata'])
         return container
@@ -116,10 +105,7 @@ def all(cls, client):
     @classmethod
     def create(cls, client, config, wait=False):
         """Create a new container config."""
-        try:
-            response = client.api.containers.post(json=config)
-        except exceptions.LXDAPIException as e:
-            raise exceptions.CreateFailed(e.response)
+        response = client.api.containers.post(json=config)
 
         if wait:
             Operation.wait_for_operation(client, response.json()['operation'])
@@ -350,13 +336,8 @@ def api(self):
 
     @classmethod
     def get(cls, client, container, name):
-        try:
-            response = client.api.containers[
-                container.name].snapshots[name].get()
-        except exceptions.LXDAPIException as e:
-            if e.response.status_code == 404:
-                raise exceptions.NotFound()
-            raise
+        response = client.api.containers[
+            container.name].snapshots[name].get()
 
         snapshot = cls(
             client, container=container,
diff --git a/pylxd/exceptions.py b/pylxd/exceptions.py
index 8fe70a1..36d8086 100644
--- a/pylxd/exceptions.py
+++ b/pylxd/exceptions.py
@@ -1,14 +1,3 @@
-class ClientConnectionFailed(Exception):
-    """An exception raised when the Client connection fails."""
-
-
-class ClientAuthenticationFailed(Exception):
-    """The LXD client's certificates are not trusted."""
-
-    def __str__(self):
-        return "LXD client certificates are not trusted."""
-
-
 class LXDAPIException(Exception):
     """A generic exception for representing unexpected LXD API responses.
 
@@ -16,6 +5,9 @@ class LXDAPIException(Exception):
     return value, and background operation, or an error. This exception
     is raised on an error case, or when the response status code is
     not expected for the response.
+
+    This exception should *only* be raised in cases where the LXD REST
+    API has returned something unexpected.
     """
 
     def __init__(self, response):
@@ -31,44 +23,5 @@ def __str__(self):
         return self.response.content.decode('utf-8')
 
 
-class _LXDAPIException(Exception):
-    """A LXD API Exception.
-
-    An exception representing an issue in the LXD API. It
-    contains the error information returned from LXD.
-
-    This exception type should be phased out, with the exception being
-    raised at a lower level (i.e. where we actually talk to the LXD
-    API, not in our pylxd logic).
-
-    DO NOT CATCH THIS EXCEPTION DIRECTLY.
-    """
-
-    def __init__(self, data):
-        self.data = data
-
-    def __str__(self):
-        return self.data.get('error')
-
-
-class NotFound(Exception):
-    """Generic NotFound exception."""
-
-    def __str__(self):
-        return 'Object not found'
-
-
-class CreateFailed(_LXDAPIException):
-    """Generic create failure exception."""
-
-
-class ObjectIncomplete(Exception):
-    """An exception raised when an object isn't completely populated.
-
-    When an object is fetched via `all`, it isn't a complete object
-    just yet. It requires a call to `fetch` to populate the object before
-    it can be pushed back up to the server.
-    """
-
-    def __str__(self):
-        return 'Object incomplete. Please call `fetch` before updating.'
+class ClientConnectionFailed(Exception):
+    """An exception raised when the Client connection fails."""
diff --git a/pylxd/image.py b/pylxd/image.py
index 795db3e..d4c70e5 100644
--- a/pylxd/image.py
+++ b/pylxd/image.py
@@ -13,7 +13,7 @@
 #    under the License.
 import hashlib
 
-from pylxd import exceptions, model
+from pylxd import model
 from pylxd.operation import Operation
 
 
@@ -40,12 +40,7 @@ def api(self):
     @classmethod
     def get(cls, client, fingerprint):
         """Get an image."""
-        try:
-            response = client.api.images[fingerprint].get()
-        except exceptions.LXDAPIException as e:
-            if e.response.status_code == 404:
-                raise exceptions.NotFound()
-            raise
+        response = client.api.images[fingerprint].get()
 
         image = cls(client, **response.json()['metadata'])
         return image
@@ -69,11 +64,8 @@ def create(cls, client, image_data, public=False, wait=False):
         headers = {}
         if public:
             headers['X-LXD-Public'] = '1'
-        try:
-            response = client.api.images.post(
-                data=image_data, headers=headers)
-        except exceptions.LXDAPIException as e:
-            raise exceptions.CreateFailed(e.response.json())
+        response = client.api.images.post(
+            data=image_data, headers=headers)
 
         if wait:
             Operation.wait_for_operation(client, response.json()['operation'])
@@ -81,11 +73,5 @@ def create(cls, client, image_data, public=False, wait=False):
 
     def export(self):
         """Export the image."""
-        try:
-            response = self.api.export.get()
-        except exceptions.LXDAPIException as e:
-            if e.response.status_code == 404:
-                raise exceptions.NotFound()
-            raise
-
+        response = self.api.export.get()
         return response.content
diff --git a/pylxd/model.py b/pylxd/model.py
index 818c1f8..9385afe 100644
--- a/pylxd/model.py
+++ b/pylxd/model.py
@@ -13,7 +13,6 @@
 #    under the License.
 import six
 
-from pylxd import exceptions
 from pylxd.deprecation import deprecated
 from pylxd.operation import Operation
 
@@ -139,12 +138,7 @@ def sync(self, rollback=False):
         """
         # XXX: rockstar (25 Jun 2016) - This has the potential to step
         # on existing attributes.
-        try:
-            response = self.api.get()
-        except exceptions.LXDAPIException as e:
-            if e.response.status_code == 404:
-                raise exceptions.NotFound()
-            raise
+        response = self.api.get()
         for key, val in response.json()['metadata'].items():
             if key not in self.__dirty__ or rollback:
                 setattr(self, key, val)
diff --git a/pylxd/profile.py b/pylxd/profile.py
index cda1015..f3c658e 100644
--- a/pylxd/profile.py
+++ b/pylxd/profile.py
@@ -11,7 +11,7 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-from pylxd import exceptions, model
+from pylxd import model
 
 
 class Profile(model.Model):
@@ -25,13 +25,7 @@ class Profile(model.Model):
     @classmethod
     def get(cls, client, name):
         """Get a profile."""
-        try:
-            response = client.api.profiles[name].get()
-        except exceptions.LXDAPIException as e:
-            if e.response.status_code == 404:
-                raise exceptions.NotFound()
-            raise
-
+        response = client.api.profiles[name].get()
         return cls(client, **response.json()['metadata'])
 
     @classmethod
@@ -53,11 +47,7 @@ def create(cls, client, name, config=None, devices=None):
             profile['config'] = config
         if devices is not None:
             profile['devices'] = devices
-        try:
-            client.api.profiles.post(json=profile)
-        except exceptions.LXDAPIException as e:
-            raise exceptions.CreateFailed(e.response.json())
-
+        client.api.profiles.post(json=profile)
         return cls.get(client, name)
 
     @property
diff --git a/pylxd/tests/test_container.py b/pylxd/tests/test_container.py
index 1085be9..b286637 100644
--- a/pylxd/tests/test_container.py
+++ b/pylxd/tests/test_container.py
@@ -24,7 +24,7 @@ def test_get(self):
         self.assertEqual(name, an_container.name)
 
     def test_get_not_found(self):
-        """NotFound is raised when the container doesn't exist."""
+        """LXDAPIException is raised when the container doesn't exist."""
         def not_found(request, context):
             context.status_code = 404
             return json.dumps({
@@ -40,7 +40,7 @@ def not_found(request, context):
         name = 'an-missing-container'
 
         self.assertRaises(
-            exceptions.NotFound,
+            exceptions.LXDAPIException,
             container.Container.get, self.client, name)
 
     def test_get_error(self):
@@ -72,25 +72,6 @@ def test_create(self):
 
         self.assertEqual(config['name'], an_new_container.name)
 
-    def test_create_failed(self):
-        """If the container creation fails, CreateFailed is raised."""
-        def create_fail(request, context):
-            context.status_code = 500
-            return json.dumps({
-                'type': 'error',
-                'error': 'An unknown error',
-                'error_code': 500})
-        self.add_rule({
-            'text': create_fail,
-            'method': 'POST',
-            'url': r'^http://pylxd.test/1.0/containers$',
-        })
-        config = {'name': 'an-new-container'}
-
-        self.assertRaises(
-            exceptions.CreateFailed,
-            container.Container.create, self.client, config)
-
     def test_fetch(self):
         """A sync updates the properties of a container."""
         an_container = container.Container(
@@ -101,7 +82,7 @@ def test_fetch(self):
         self.assertTrue(an_container.ephemeral)
 
     def test_fetch_not_found(self):
-        """NotFound is raised on a 404 for updating container."""
+        """LXDAPIException is raised on a 404 for updating container."""
         def not_found(request, context):
             context.status_code = 404
             return json.dumps({
@@ -117,7 +98,7 @@ def not_found(request, context):
         an_container = container.Container(
             self.client, name='an-missing-container')
 
-        self.assertRaises(exceptions.NotFound, an_container.sync)
+        self.assertRaises(exceptions.LXDAPIException, an_container.sync)
 
     def test_fetch_error(self):
         """LXDAPIException is raised on error."""
@@ -355,7 +336,7 @@ def test_get(self):
         self.assertEqual(b'This is a getted file', data)
 
     def test_get_not_found(self):
-        """NotFound is raised on bogus filenames."""
+        """LXDAPIException is raised on bogus filenames."""
         def not_found(request, context):
             context.status_code = 500
         rule = {
@@ -366,7 +347,7 @@ def not_found(request, context):
         self.add_rule(rule)
 
         self.assertRaises(
-            exceptions.NotFound, self.container.files.get, '/tmp/getted')
+            exceptions.LXDAPIException, self.container.files.get, '/tmp/getted')
 
     def test_get_error(self):
         """LXDAPIException is raised on error."""
diff --git a/pylxd/tests/test_image.py b/pylxd/tests/test_image.py
index d889553..9c3686e 100644
--- a/pylxd/tests/test_image.py
+++ b/pylxd/tests/test_image.py
@@ -16,7 +16,7 @@ def test_get(self):
         self.assertEqual(fingerprint, a_image.fingerprint)
 
     def test_get_not_found(self):
-        """NotFound is raised when the image isn't found."""
+        """LXDAPIException is raised when the image isn't found."""
         def not_found(request, context):
             context.status_code = 404
             return json.dumps({
@@ -32,7 +32,7 @@ def not_found(request, context):
         fingerprint = hashlib.sha256(b'').hexdigest()
 
         self.assertRaises(
-            exceptions.NotFound,
+            exceptions.LXDAPIException,
             image.Image.get, self.client, fingerprint)
 
     def test_get_error(self):
@@ -69,24 +69,6 @@ def test_create(self):
         self.assertIsInstance(a_image, image.Image)
         self.assertEqual(fingerprint, a_image.fingerprint)
 
-    def test_create_failed(self):
-        """If image creation fails, CreateFailed is raised."""
-        def create_fail(request, context):
-            context.status_code = 500
-            return json.dumps({
-                'type': 'error',
-                'error': 'An unknown error',
-                'error_code': 500})
-        self.add_rule({
-            'text': create_fail,
-            'method': 'POST',
-            'url': r'^http://pylxd.test/1.0/images$',
-        })
-
-        self.assertRaises(
-            exceptions.CreateFailed,
-            image.Image.create, self.client, b'')
-
     def test_update(self):
         """An image is updated."""
         a_image = self.client.images.all()[0]
@@ -103,7 +85,7 @@ def test_fetch(self):
         self.assertEqual(1, a_image.size)
 
     def test_fetch_notfound(self):
-        """A bogus image fetch raises NotFound."""
+        """A bogus image fetch raises LXDAPIException."""
         def not_found(request, context):
             context.status_code = 404
             return json.dumps({
@@ -119,7 +101,7 @@ def not_found(request, context):
 
         a_image = image.Image(self.client, fingerprint=fingerprint)
 
-        self.assertRaises(exceptions.NotFound, a_image.sync)
+        self.assertRaises(exceptions.LXDAPIException, a_image.sync)
 
     def test_fetch_error(self):
         """A 500 error raises LXDAPIException."""
@@ -159,7 +141,7 @@ def test_export(self):
         self.assertEqual(a_image.fingerprint, data_sha)
 
     def test_export_not_found(self):
-        """NotFound is raised on export of bogus image."""
+        """LXDAPIException is raised on export of bogus image."""
         def not_found(request, context):
             context.status_code = 404
             return json.dumps({
@@ -173,7 +155,7 @@ def not_found(request, context):
         })
         a_image = self.client.images.all()[0]
 
-        self.assertRaises(exceptions.NotFound, a_image.export)
+        self.assertRaises(exceptions.LXDAPIException, a_image.export)
 
     def test_export_error(self):
         """LXDAPIException is raised on API error."""
diff --git a/pylxd/tests/test_profile.py b/pylxd/tests/test_profile.py
index 1e77b65..c8f4e6c 100644
--- a/pylxd/tests/test_profile.py
+++ b/pylxd/tests/test_profile.py
@@ -15,7 +15,7 @@ def test_get(self):
         self.assertEqual(name, an_profile.name)
 
     def test_get_not_found(self):
-        """NotFound is raised on unknown profiles."""
+        """LXDAPIException is raised on unknown profiles."""
         def not_found(request, context):
             context.status_code = 404
             return json.dumps({
@@ -29,7 +29,7 @@ def not_found(request, context):
         })
 
         self.assertRaises(
-            exceptions.NotFound,
+            exceptions.LXDAPIException,
             profile.Profile.get, self.client, 'an-profile')
 
     def test_get_error(self):
@@ -72,25 +72,6 @@ def test_rename(self):
 
         self.assertEqual('an-renamed-profile', an_renamed_profile.name)
 
-    def test_create_failed(self):
-        """CreateFailed is raised when errors occur."""
-        def error(request, context):
-            context.status_code = 503
-            return json.dumps({
-                'type': 'error',
-                'error': 'An unknown error',
-                'error_code': 500})
-        self.add_rule({
-            'text': error,
-            'method': 'POST',
-            'url': r'^http://pylxd.test/1.0/profiles$',
-        })
-
-        self.assertRaises(
-            exceptions.CreateFailed,
-            profile.Profile.create, self.client,
-            name='an-new-profile', config={})
-
     def test_update(self):
         """A profile is updated."""
         # XXX: rockstar (03 Jun 2016) - This just executes
@@ -111,7 +92,7 @@ def test_fetch(self):
         self.assertEqual('An description', an_profile.description)
 
     def test_fetch_notfound(self):
-        """NotFound is raised on bogus profile fetches."""
+        """LXDAPIException is raised on bogus profile fetches."""
         def not_found(request, context):
             context.status_code = 404
             return json.dumps({
@@ -126,7 +107,7 @@ def not_found(request, context):
 
         an_profile = profile.Profile(self.client, name='an-profile')
 
-        self.assertRaises(exceptions.NotFound, an_profile.sync)
+        self.assertRaises(exceptions.LXDAPIException, an_profile.sync)
 
     def test_fetch_error(self):
         """LXDAPIException is raised on fetch error."""

From d06643d0dd89b045cd6407914dddee933fdb32b9 Mon Sep 17 00:00:00 2001
From: Paul Hummer <paul.hummer at canonical.com>
Date: Tue, 28 Jun 2016 13:21:41 -0600
Subject: [PATCH 2/3] Fix for 2.0.3

---
 pylxd/client.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pylxd/client.py b/pylxd/client.py
index 81a278c..b9a0cca 100644
--- a/pylxd/client.py
+++ b/pylxd/client.py
@@ -83,7 +83,9 @@ def get(self, *args, **kwargs):
     def post(self, *args, **kwargs):
         """Perform an HTTP POST."""
         response = self.session.post(self._api_endpoint, *args, **kwargs)
-        self._assert_response(response, allowed_status_codes=(200, 202))
+        # Prior to LXD 2.0.3, successful synchronous requests returned 200,
+        # rather than 201.
+        self._assert_response(response, allowed_status_codes=(200, 201, 202))
         return response
 
     def put(self, *args, **kwargs):

From 22889452e164df7d8d7b0cc47a62f98e1ea4eaed Mon Sep 17 00:00:00 2001
From: Paul Hummer <paul.hummer at canonical.com>
Date: Tue, 28 Jun 2016 13:22:51 -0600
Subject: [PATCH 3/3] Fix lint

---
 pylxd/tests/test_container.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pylxd/tests/test_container.py b/pylxd/tests/test_container.py
index b286637..d483cba 100644
--- a/pylxd/tests/test_container.py
+++ b/pylxd/tests/test_container.py
@@ -347,7 +347,8 @@ def not_found(request, context):
         self.add_rule(rule)
 
         self.assertRaises(
-            exceptions.LXDAPIException, self.container.files.get, '/tmp/getted')
+            exceptions.LXDAPIException,
+            self.container.files.get, '/tmp/getted')
 
     def test_get_error(self):
         """LXDAPIException is raised on error."""


More information about the lxc-devel mailing list