From 7d80786532847650cadf51d1a1935a65f22fb1d6 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jun 09 2017 09:50:03 +0000 Subject: [PATCH 1/2] Fix incorrect lightblue entity name Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 764cb45..4c75d13 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -173,7 +173,7 @@ class LightBlue(object): """ url = 'find/containerRepository/{}'.format( - self._get_entity_version('entityRespository')) + self._get_entity_version('containerRepository')) response = self._make_request(url, request) repos = [] diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 11dce78..6fea457 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -520,40 +520,35 @@ class TestEntityVersion(unittest.TestCase): self.fake_cert_file = 'path/to/cert' self.fake_private_key = 'path/to/private-key' self.fake_entity_versions = { - 'containerImage': '0.0.11' + 'containerImage': '0.0.11', + 'containerRepository': '0.0.12', } @patch('freshmaker.lightblue.LightBlue._make_request') - @patch('os.path.exists') - def test_use_default_entity_version(self, exists, _make_request): - exists.return_value = True - + @patch('os.path.exists', return_value=True) + def test_use_specified_container_image_version(self, exists, _make_request): lb = LightBlue(server_url=self.fake_server_url, cert=self.fake_cert_file, private_key=self.fake_private_key, entity_versions=self.fake_entity_versions) - lb.find_container_repositories({}) + lb.find_container_images({}) - _make_request.assert_called_once_with('find/containerRepository/', {}) + _make_request.assert_called_once_with('find/containerImage/0.0.11', {}) @patch('freshmaker.lightblue.LightBlue._make_request') - @patch('os.path.exists') - def test_use_specified_entity_version(self, exists, _make_request): - exists.return_value = True - + @patch('os.path.exists', return_value=True) + def test_use_specified_container_repository_version(self, exists, _make_request): lb = LightBlue(server_url=self.fake_server_url, cert=self.fake_cert_file, private_key=self.fake_private_key, entity_versions=self.fake_entity_versions) - lb.find_container_images({}) + lb.find_container_repositories({}) - _make_request.assert_called_once_with('find/containerImage/0.0.11', {}) + _make_request.assert_called_once_with('find/containerRepository/0.0.12', {}) @patch('freshmaker.lightblue.LightBlue._make_request') - @patch('os.path.exists') - def test_use_default_entity_version_when_parameter_is_omitted( - self, exists, _make_request): - exists.return_value = True + @patch('os.path.exists', return_value=True) + def test_use_default_entity_version(self, exists, _make_request): _make_request.return_value = { # Omit other attributes that are not useful for this test 'processed': [] From f86adaa20c87893f232471232ee6af7c6abe2438 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jun 12 2017 03:24:00 +0000 Subject: [PATCH 2/2] Handle LightBlue non-JSON errors Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 4c75d13..1b26187 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -21,49 +21,70 @@ # # Written by Chenxiong Qi +import json import os +import re import requests -import json +import six from six.moves import http_client -class LightBlueRequestFailure(Exception): - """Exception when fail to request from LightBlue""" +class LightBlueError(Exception): + """Base class representing errors from LightBlue server""" - def __init__(self, json_response, status_code): + def __init__(self, status_code, error_response): """Initialize - :param dict json_response: the JSON data returned from LightBlue - which contains all error information. :param int status_code: repsonse status code + :param str or dict error_response: response content returned from + LightBlue server that contains error content. There are two types of + error. A piece of HTML when error happens in system-wide, for example, + requested resource does not exists (404), and internal server error (500). + It could also be a JSON data when error happens while LightBlue handles + request. """ - self._raw = json_response + self._raw = error_response self._status_code = status_code def __repr__(self): return '<{} [{}]>'.format(self.__class__.__name__, self.status_code) - def __str__(self): - return 'Error{} ({}):\n{}'.format( - 's' if len(self.errors) > 1 else '', - len(self.errors), - '\n'.join((' {}'.format(err['msg']) for err in self.errors)) - ) - @property def raw(self): return self._raw @property - def errors(self): - return self.raw['errors'] - - @property def status_code(self): return self._status_code +class LightBlueSystemError(LightBlueError): + """LightBlue system error""" + + def _get_error_message(self): + # Remove all newlines if there is + buf = six.StringIO(self.raw) + html = ''.join((line.strip('\n') for line in buf)) + match = re.search('(.+)', html) + return match.groups()[0] + + def __str__(self): + return self._get_error_message() + + +class LightBlueRequestError(LightBlueError): + """LightBlue request error""" + + def __str__(self): + return 'Error{} ({}):\n{}'.format( + 's' if len(self.raw['errors']) > 1 else '', + len(self.raw['errors']), + '\n'.join((' {}'.format(err['msg']) + for err in self.raw['errors'])) + ) + + class ContainerRepository(dict): """Represent a container repository""" @@ -154,12 +175,20 @@ class LightBlue(object): :param dict response: the response returned from LightBlue, which is actually the requests response object. - :raises LightBlueRequestFailure: if response status code is not 200. - Otherwise, just keep silient. + :raises LightBlueSystemError or LightBlueRequestError: if response + status code is not 200. Otherwise, just keep silient. """ - if response.status_code == http_client.OK: + status_code = response.status_code + + if status_code == http_client.OK: return - raise LightBlueRequestFailure(response.json(), response.status_code) + + if status_code in (http_client.NOT_FOUND, + http_client.INTERNAL_SERVER_ERROR, + http_client.UNAUTHORIZED): + raise LightBlueSystemError(status_code, response.content) + + raise LightBlueRequestError(status_code, response.json()) def find_container_repositories(self, request): """Query via entity containerRepository diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 6fea457..1c2b85e 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -21,6 +21,7 @@ # SOFTWARE. import json +import six import unittest from mock import call, patch @@ -29,11 +30,12 @@ from six.moves import http_client from freshmaker.lightblue import ContainerImage from freshmaker.lightblue import ContainerRepository from freshmaker.lightblue import LightBlue -from freshmaker.lightblue import LightBlueRequestFailure +from freshmaker.lightblue import LightBlueRequestError +from freshmaker.lightblue import LightBlueSystemError -class TestLightBlueRequestFailure(unittest.TestCase): - """Test case for exception LightBlueRequestFailure""" +class TestLightBlueRequestError(unittest.TestCase): + """Test case for exception LightBlueRequestError""" def setUp(self): self.fake_error_data = { @@ -53,17 +55,17 @@ class TestLightBlueRequestFailure(unittest.TestCase): 'modifiedCount': 0, 'status': 'ERROR' } - self.e = LightBlueRequestFailure(self.fake_error_data, - http_client.INTERNAL_SERVER_ERROR) + self.e = LightBlueRequestError(http_client.BAD_REQUEST, + self.fake_error_data) def test_get_raw_error_json_data(self): self.assertEqual(self.fake_error_data, self.e.raw) def test_get_status_code(self): - self.assertEqual(http_client.INTERNAL_SERVER_ERROR, self.e.status_code) + self.assertEqual(http_client.BAD_REQUEST, self.e.status_code) def test_get_inner_errors(self): - self.assertEqual(self.fake_error_data['errors'], self.e.errors) + self.assertEqual(self.fake_error_data['errors'], self.e.raw['errors']) def test_errors_listed_in_str(self): expected_s = '\n'.join((' {}'.format(err['msg']) @@ -71,6 +73,50 @@ class TestLightBlueRequestFailure(unittest.TestCase): self.assertIn(expected_s, str(self.e)) +class TestLightBlueSystemError(unittest.TestCase): + """Test LightBlueSystemError""" + + def setUp(self): + buf = six.StringIO(''' +JBWEB000065: HTTP Status 401 - JBWEB000009: No client +certificate chain in this request

+JBWEB000065: HTTP Status 401 - JBWEB000009: No client certificate chain in +this request

JBWEB000309: type +JBWEB000067: Status report

JBWEB000068: message JBWEB000009: +No client certificate chain in this request

JBWEB000069: +description JBWEB000121: This request requires HTTP authentication. +

+''') + self.fake_error_data = ' '.join((line.strip() for line in buf)) + self.e = LightBlueSystemError(http_client.UNAUTHORIZED, + self.fake_error_data) + + def test_get_status_code(self): + self.assertEqual(http_client.UNAUTHORIZED, self.e.status_code) + + def test_raw(self): + self.assertEqual(self.fake_error_data, self.e.raw) + + def test__str__(self): + self.assertEqual( + 'JBWEB000065: HTTP Status 401 - JBWEB000009: No client certificate' + ' chain in this request', + str(self.e)) + + def test__repr__(self): + self.assertEqual('<{} [{}]>'.format(self.e.__class__.__name__, + self.e.status_code), + repr(self.e)) + + class TestContainerImageObject(unittest.TestCase): def test_create(self): @@ -303,7 +349,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): @patch('freshmaker.lightblue.requests.post') def test_raise_error_if_request_data_is_incorrect(self, post): - post.return_value.status_code = http_client.INTERNAL_SERVER_ERROR + post.return_value.status_code = http_client.BAD_REQUEST post.return_value.json.return_value = { 'entity': 'containerImage', 'entityVersion': '0.0.11', @@ -333,7 +379,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): lb = LightBlue(server_url=self.fake_server_url, cert=self.fake_cert_file, private_key=self.fake_private_key) - self.assertRaises(LightBlueRequestFailure, + self.assertRaises(LightBlueRequestError, lb._make_request, 'find/containerRepository/', fake_request) @patch('freshmaker.lightblue.LightBlue.find_container_repositories') @@ -490,23 +536,23 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): cont_repos): exists.return_value = True - cont_repos.side_effect = LightBlueRequestFailure( + cont_repos.side_effect = LightBlueRequestError( {"errors": [{"msg": "dummy error"}]}, http_client.REQUEST_TIMEOUT) cont_images.return_value = self.fake_images_with_parsed_data lb = LightBlue(server_url=self.fake_server_url, cert=self.fake_cert_file, private_key=self.fake_private_key) - with self.assertRaises(LightBlueRequestFailure): + with self.assertRaises(LightBlueRequestError): lb.find_images_with_package_from_content_set( "openssl", ["dummy-content-set-1"]) cont_repos.return_value = self.fake_repositories_with_content_sets - cont_images.side_effect = LightBlueRequestFailure( + cont_images.side_effect = LightBlueRequestError( {"errors": [{"msg": "dummy error"}]}, http_client.REQUEST_TIMEOUT) - with self.assertRaises(LightBlueRequestFailure): + with self.assertRaises(LightBlueRequestError): lb.find_images_with_package_from_content_set( "openssl", ["dummy-content-set-1"])