The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/pylxd/pull/153
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
- Remove the extra exceptions that aren't really needed with the introduction of `LXDAPIException`.
- Fix the assertions of responses for POST, to support LXD 2.0.3
From 50aa3095f1c72a80aa91a3a6cd21d529903e7e10 Mon Sep 17 00:00:00 2001
From: Paul Hummer <paul.hum...@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.hum...@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.hum...@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."""
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
