PR#28: Add support to query LightBlue - freshmaker

freshmaker

#28 Add support to query LightBlue

Merged 8 years ago by jkaluza. Opened 8 years ago by cqi.

cqi/freshmaker query-lightblue into master

Make entity versions configurable

Chenxiong Qi • 8 years ago

b8fa14b

Add support to query LightBlue

Chenxiong Qi • 8 years ago

a2bf91e

conf/config.py

file modified

+14

		`@@ -120,6 +120,15 @@`
		`# },`
		`# }`

		`+ LIGHTBLUE_SERVER_URL = '' # replace with default server url`
		`+ LIGHTBLUE_VERIFY_SSL = True`
		`+`
		`+ # Lookup versions of each entity: /rest/metadata/{entity name}`
		`+ LIGHTBLUE_ENTITY_VERSIONS = {`
		`+ 'containerRepository': '0.0.11',`
		`+ 'containerImage': '0.0.12',`
		`+ }`
		`+`

		`class DevConfiguration(BaseConfiguration):`
		`DEBUG = True`
		`@@ -134,6 +143,8 @@`

		`KOJI_CONTAINER_SCRATCH_BUILD = True`

		`+ LIGHTBLUE_VERIFY_SSL = False`
		`+`

		`class TestConfiguration(BaseConfiguration):`
		`LOG_BACKEND = 'console'`
		`@@ -153,6 +164,9 @@`

		`KOJI_CONTAINER_SCRATCH_BUILD = True`

		`+ LIGHTBLUE_SERVER_URL = '' # replace with real dev server url`
		`+ LIGHTBLUE_VERIFY_SSL = False`
		`+`

		`class ProdConfiguration(BaseConfiguration):`
		`pass`

freshmaker/config.py

file modified

		`@@ -186,6 +186,14 @@`
		`'default': {},`
		`'desc': 'Blacklist for build targets of handlers',`
		`},`
		`+ 'lightblue_server_url': {`
		`+ 'type': str,`
		`+ 'default': '',`
		`+ 'desc': 'Server URL of LightBlue.'},`
		`+ 'lightblue_verify_ssl': {`
		`+ 'type': bool,`
		`+ 'default': True,`
		`+ 'desc': 'Whether to enable SSL verification over HTTP with lightblue.'},`
		`}`

		`def __init__(self, conf_section_obj):`

freshmaker/lightblue.py

file added

+206

		`@@ -0,0 +1,206 @@`
		`+ # -- coding: utf-8 --`
		`+ # Copyright (c) 2017 Red Hat, Inc.`
		`+ #`
		`+ # Permission is hereby granted, free of charge, to any person obtaining a copy`
		`+ # of this software and associated documentation files (the "Software"), to deal`
		`+ # in the Software without restriction, including without limitation the rights`
		`+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell`
		`+ # copies of the Software, and to permit persons to whom the Software is`
		`+ # furnished to do so, subject to the following conditions:`
		`+ #`
		`+ # The above copyright notice and this permission notice shall be included in`
		`+ # all copies or substantial portions of the Software.`
		`+ #`
		`+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR`
		`+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,`
		`+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE`
		`+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER`
		`+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,`
		`+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`+ # SOFTWARE.`
		`+ #`
		`+ # Written by Chenxiong Qi <cqi@redhat.com>`
		`+`
		`+ import os`
		`+ import requests`
		`+ import json`
		`+`
		`+ from six.moves import http_client`
		`+`
		`+`
		`+ class LightBlueRequestFailure(Exception):`
		`+ """Exception when fail to request from LightBlue"""`
		`+`
		`+ def __init__(self, json_response, status_code):`
		`+ """Initialize`
		`+`
		`+ :param dict json_response: the JSON data returned from LightBlue`
		`+ which contains all error information.`
		`+ :param int status_code: repsonse status code`
		`+ """`
		`+ self._raw = json_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 ContainerRepository(dict):`
		`+ """Represent a container repository"""`
		`+`
		`+ @classmethod`
		`+ def create(cls, data):`
		`+ repo = cls()`
		`+ repo.update(data)`
		`+ return repo`
		`+`
		`+`
		`+ class ContainerImage(dict):`
		`+ """Represent a container image"""`
		`+`
		`+ @classmethod`
		`+ def create(cls, data):`
		`+ image = cls()`
		`+ image.update(data)`
		`+ return image`
		`+`
		`+`
		`+ class LightBlue(object):`
		`+ """Interface to query lightblue"""`
		`+`
		`+ def __init__(self, server_url, cert, private_key,`
		`+ verify_ssl=None,`
		`+ entity_versions=None):`
		`+ """Initialize LightBlue instance`
		`+`
		`+ :param str server_url: URL used to call LightBlue APIs. It is`
		`+ unnecessary to include path part, which will be handled`
		`+ automatically. For example, https://lightblue.example.com/.`
		`+ :param str cert: path to certificate file.`
		`+ :param str private_key: path to private key file.`
		`+ :param bool verify_ssl: whether to verify SSL over HTTP. Enabled by`
		`+ default.`
		`+ :param dict entity_versions: a mapping from entity to what version`
		`+ should be used to request data. If no such a mapping appear , it`
		`+ means the default version will be used. You should choose versions`
		`+ explicitly. If entity_versions is omitted entirely, default version`
		`+ will be used on each entity.`
		`+ """`
		`+ self.server_url = server_url`
		`+ self.api_root = '{}/rest/data/'.format(server_url)`
		`+ if verify_ssl is None:`
		`+ self.verify_ssl = True`
		`+ else:`
		`+ assert isinstance(verify_ssl, bool)`
		`+ self.verify_ssl = verify_ssl`
		`+`
		`+ if not os.path.exists(cert):`
		`+ raise IOError('Certificate file {} does not exist.'.format(cert))`
		`+ else:`
		`+ self.cert = cert`
		`+`
		`+ if not os.path.exists(private_key):`
		`+ raise IOError('Private key file {} does not exist.'.format(private_key))`
		`+ else:`
		`+ self.private_key = private_key`
		`+`
		`+ self.entity_versions = entity_versions or {}`
		`+`
		`+ def _get_entity_version(self, entity_name):`
		`+ """Lookup configured entity's version`
		`+`
		`+ :param str entity_name: entity name to get its version.`
		`+ :return: version configured for the entity name. If there is no`
		`+ corresponding version, emtpy string is returned, which can be used`
		`+ to construct request URL directly that means to use default`
		`+ version.`
		`+ :rtype: str`
		`+ """`
		`+ return self.entity_versions.get(entity_name, '')`
		`+`
		`+ def _make_request(self, entity, data):`
		`+ """Make request to lightblue"""`
		`+`
		`+ entity_url = '{}/{}'.format(self.api_root, entity)`
		`+ response = requests.post(entity_url,`
		`+ data=json.dumps(data),`
		`+ verify=self.verify_ssl,`
		`+ cert=(self.cert, self.private_key),`
		`+ headers={'Content-Type': 'application/json'})`
		`+ self._raise_expcetion_if_errors_returned(response)`
		`+ return response.json()`
		`+`
		`+ def _raise_expcetion_if_errors_returned(self, response):`
		`+ """Raise exception when response contains errors`
		`+`
		`+ :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.`
		`+ """`
		`+ if response.status_code == http_client.OK:`
		`+ return`
		`+ raise LightBlueRequestFailure(response.json(), response.status_code)`
		`+`
		`+ def find_container_repositories(self, request):`
		`+ """Query via entity containerRepository`
		`+`
		`+ :param dict request: a map containing complete query expression.`
		`+ This query will be sent to LightBlue in a POST request. Refer to`
		`+ https://jewzaam.gitbooks.io/lightblue-specifications/content/language_specification/query.html`
		`+ to know more detail about how to write a query.`
		`+ :return: a list of ContainerRepository objects`
		`+ :rtype: list`
		`+ """`
		`+`
		`+ url = 'find/containerRepository/{}'.format(`
		`+ self._get_entity_version('entityRespository'))`
		`+ response = self._make_request(url, request)`
		`+`
		`+ repos = []`
		`+ for repo_data in response['processed']:`
		`+ repo = ContainerRepository()`
		`+ repo.update(repo_data)`
		`+ repos.append(repo)`
		`+ return repos`
		`+`
		`+ def find_container_images(self, request):`
		`+ """Query via entity containerImage`
		`+`
		`+ :param dict request: a map containing complete query expression.`
		`+ This query will be sent to LightBlue in a POST request. Refer to`
		`+ https://jewzaam.gitbooks.io/lightblue-specifications/content/language_specification/query.html`
		`+ to know more detail about how to write a query.`
		`+ :return: a list of ContainerImage objects`
		`+ :rtype: list`
		`+ """`
		`+`
		`+ url = 'find/containerImage/{}'.format(`
		`+ self._get_entity_version('containerImage'))`
		`+ response = self._make_request(url, request)`
		`+`
		`+ images = []`
		`+ for image_data in response['processed']:`
		`+ image = ContainerImage()`
		`+ image.update(image_data)`
		`+ images.append(image)`
		`+ return images`

requirements.txt

file modified

		`@@ -20,3 +20,4 @@`
		`Flask-Migrate`
		`Flask-SQLAlchemy`
		`Flask-Script`
		`+ requests`

tests/test_lightblue.py

file added

+329

		`@@ -0,0 +1,329 @@`
		`+ # -- coding: utf-8 --`
		`+ #`
		`+ # Copyright (c) 2017 Red Hat, Inc.`
		`+ #`
		`+ # Permission is hereby granted, free of charge, to any person obtaining a copy`
		`+ # of this software and associated documentation files (the "Software"), to deal`
		`+ # in the Software without restriction, including without limitation the rights`
		`+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell`
		`+ # copies of the Software, and to permit persons to whom the Software is`
		`+ # furnished to do so, subject to the following conditions:`
		`+ #`
		`+ # The above copyright notice and this permission notice shall be included in all`
		`+ # copies or substantial portions of the Software.`
		`+ #`
		`+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR`
		`+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,`
		`+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE`
		`+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER`
		`+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,`
		`+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`+ # SOFTWARE.`
		`+`
		`+ import json`
		`+ import unittest`
		`+`
		`+ from mock import call`
		`+ from mock import patch`
		`+ 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`
		`+`
		`+`
		`+ class TestLightBlueRequestFailure(unittest.TestCase):`
		`+ """Test case for exception LightBlueRequestFailure"""`
		`+`
		`+ def setUp(self):`
		`+ self.fake_error_data = {`
		`+ 'entity': 'containerImage',`
		`+ 'entityVersion': '0.0.11',`
		`+ 'errors': [`
		`+ {`
		`+ 'context': 'rest/FindCommand/containerImage/find(containerImage:0.0.11)/'`
		`+ 'containerImage/parsed_data/rpm_manifes',`
		`+ 'errorCode': 'metadata:InvalidFieldReference',`
		`+ 'msg': 'rpm_manifes in parsed_data.rpm_manifes.*.nvra',`
		`+ 'objectType': 'error'`
		`+ }`
		`+ ],`
		`+ 'hostname': 'lightbluecrud1.dev2.a1.vary.redhat.com',`
		`+ 'matchCount': 0,`
		`+ 'modifiedCount': 0,`
		`+ 'status': 'ERROR'`
		`+ }`
		`+ self.e = LightBlueRequestFailure(self.fake_error_data,`
		`+ http_client.INTERNAL_SERVER_ERROR)`
		`+`
		`+ 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)`
		`+`
		`+ def test_get_inner_errors(self):`
		`+ self.assertEqual(self.fake_error_data['errors'], self.e.errors)`
		`+`
		`+ def test_errors_listed_in_str(self):`
		`+ expected_s = '\n'.join((' {}'.format(err['msg'])`
		`+ for err in self.fake_error_data['errors']))`
		`+ self.assertIn(expected_s, str(self.e))`
		`+`
		`+`
		`+ class TestContainerImageObject(unittest.TestCase):`
		`+`
		`+ def test_create(self):`
		`+ image = ContainerImage.create({`
		`+ '_id': '1233829',`
		`+ 'brew': {`
		`+ 'completion_date': '20151210T10:09:35.000-0500',`
		`+ 'build': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker-1.1-6',`
		`+ 'package': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker'`
		`+ }`
		`+ })`
		`+`
		`+ self.assertEqual('1233829', image['_id'])`
		`+ self.assertEqual('20151210T10:09:35.000-0500', image['brew']['completion_date'])`
		`+`
		`+`
		`+ class TestContainerRepository(unittest.TestCase):`
		`+`
		`+ def test_create(self):`
		`+ image = ContainerRepository.create({`
		`+ 'creationDate': '20160927T11:14:56.420-0400',`
		`+ 'metrics': {`
		`+ 'pulls_in_last_30_days': 0,`
		`+ 'last_update_date': '20170223T08:28:40.913-0500'`
		`+ }`
		`+ })`
		`+`
		`+ self.assertEqual('20160927T11:14:56.420-0400', image['creationDate'])`
		`+ self.assertEqual(0, image['metrics']['pulls_in_last_30_days'])`
		`+ self.assertEqual('20170223T08:28:40.913-0500', image['metrics']['last_update_date'])`
		`+`
		`+`
		`+ class TestQueryEntityFromLightBlue(unittest.TestCase):`
		`+`
		`+ def setUp(self):`
		`+ self.fake_server_url = 'lightblue.localhost'`
		`+ self.fake_cert_file = 'path/to/cert'`
		`+ self.fake_private_key = 'path/to/private-key'`
		`+`
		`+ @patch('freshmaker.lightblue.requests.post')`
		`+ def test_find_container_images(self, post):`
		`+ post.return_value.status_code = http_client.OK`
		`+ post.return_value.json.return_value = {`
		`+ 'modifiedCount': 0,`
		`+ 'resultMetadata': [],`
		`+ 'entityVersion': '0.0.12',`
		`+ 'hostname': self.fake_server_url,`
		`+ 'matchCount': 2,`
		`+ 'processed': [`
		`+ {`
		`+ '_id': '57ea8d1f9c624c035f96f4b0',`
		`+ 'image_id': 'e0f97342ddf6a09972434f98837b5fd8b5bed9390f32f1d63e8a7e4893208af7',`
		`+ 'brew': {`
		`+ 'completion_date': '20151210T10:09:35.000-0500',`
		`+ 'build': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker-1.1-6',`
		`+ 'package': 'jboss-webserver-3-webserver30-tomcat7-openshift-docker'`
		`+ },`
		`+ },`
		`+ {`
		`+ '_id': '57ea8d289c624c035f96f4db',`
		`+ 'image_id': 'c1ef3345f36b901b0bddc7ab01ea3f3c83c886faa243e02553f475124eb4b46c',`
		`+ 'brew': {`
		`+ 'package': 'sadc-docker',`
		`+ 'completion_date': '20151203T00:35:30.000-0500',`
		`+ 'build': 'sadc-docker-7.2-7'`
		`+ },`
		`+ }`
		`+ ],`
		`+ 'status': 'COMPLETE',`
		`+ 'entity': 'containerImage'`
		`+ }`
		`+`
		`+ fake_request = {`
		`+ "objectType": "containerImage",`
		`+ "projection": [`
		`+ {"field": "_id", "include": True},`
		`+ {"field": "image_id", "include": True},`
		`+ {"field": "brew", "include": True, "recursive": True},`
		`+ ],`
		`+ }`
		`+`
		`+ with patch('os.path.exists'):`
		`+ lb = LightBlue(server_url=self.fake_server_url,`
		`+ cert=self.fake_cert_file,`
		`+ private_key=self.fake_private_key)`
		`+ images = lb.find_container_images(request=fake_request)`
		`+`
		`+ post.assert_called_once_with(`
		`+ '{}/{}/'.format(lb.api_root, 'find/containerImage'),`
		`+ data=json.dumps(fake_request),`
		`+ verify=lb.verify_ssl,`
		`+ cert=(self.fake_cert_file, self.fake_private_key),`
		`+ headers={'Content-Type': 'application/json'}`
		`+ )`
		`+ self.assertEqual(2, len(images))`
		`+`
		`+ image = images[0]`
		`+ self.assertEqual('57ea8d1f9c624c035f96f4b0', image['_id'])`
		`+ self.assertEqual('jboss-webserver-3-webserver30-tomcat7-openshift-docker',`
		`+ image['brew']['package'])`
		`+`
		`+ @patch('freshmaker.lightblue.requests.post')`
		`+ def test_find_container_repositories(self, post):`
		`+ post.return_value.status_code = http_client.OK`
		`+ post.return_value.json.return_value = {`
		`+ 'entity': 'containerRepository',`
		`+ 'status': 'COMPLETE',`
		`+ 'modifiedCount': 0,`
		`+ 'matchCount': 2,`
		`+ 'processed': [`
		`+ {`
		`+ 'creationDate': '20160927T11:14:56.420-0400',`
		`+ 'metrics': {`
		`+ 'pulls_in_last_30_days': 0,`
		`+ 'last_update_date': '20170223T08:28:40.913-0500'`
		`+ }`
		`+ },`
		`+ {`
		`+ 'creationDate': '20161020T04:52:43.365-0400',`
		`+ 'metrics': {`
		`+ 'last_update_date': '20170501T03:00:19.892-0400',`
		`+ 'pulls_in_last_30_days': 20`
		`+ }`
		`+ }`
		`+ ],`
		`+ 'entityVersion': '0.0.11',`
		`+ 'hostname': self.fake_server_url,`
		`+ 'resultMetadata': []`
		`+ }`
		`+`
		`+ fake_request = {`
		`+ "objectType": "containerRepository",`
		`+ "projection": [`
		`+ {"field": "creationDate", "include": True},`
		`+ {"field": "metrics", "include": True, "recursive": True}`
		`+ ],`
		`+ }`
		`+`
		`+ with patch('os.path.exists'):`
		`+ lb = LightBlue(server_url=self.fake_server_url,`
		`+ cert=self.fake_cert_file,`
		`+ private_key=self.fake_private_key)`
		`+ repos = lb.find_container_repositories(request=fake_request)`
		`+`
		`+ post.assert_called_once_with(`
		`+ '{}/{}/'.format(lb.api_root, 'find/containerRepository'),`
		`+ data=json.dumps(fake_request),`
		`+ verify=lb.verify_ssl,`
		`+ cert=(self.fake_cert_file, self.fake_private_key),`
		`+ headers={'Content-Type': 'application/json'}`
		`+ )`
		`+`
		`+ self.assertEqual(2, len(repos))`
		`+`
		`+ repo = repos[0]`
		`+ self.assertEqual('20160927T11:14:56.420-0400', repo['creationDate'])`
		`+ self.assertEqual(0, repo['metrics']['pulls_in_last_30_days'])`
		`+ self.assertEqual('20170223T08:28:40.913-0500', repo['metrics']['last_update_date'])`
		`+`
		`+ @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.json.return_value = {`
		`+ 'entity': 'containerImage',`
		`+ 'entityVersion': '0.0.11',`
		`+ 'errors': [`
		`+ {`
		`+ 'context': 'rest/FindCommand/containerImage/find(containerImage:0.0.11)/'`
		`+ 'containerImage/parsed_data/rpm_manifes',`
		`+ 'errorCode': 'metadata:InvalidFieldReference',`
		`+ 'msg': 'rpm_manifes in parsed_data.rpm_manifes.*.nvra',`
		`+ 'objectType': 'error'`
		`+ }`
		`+ ],`
		`+ 'hostname': 'lightbluecrud1.dev2.a1.vary.redhat.com',`
		`+ 'matchCount': 0,`
		`+ 'modifiedCount': 0,`
		`+ 'status': 'ERROR'`
		`+ }`
		`+`
		`+ fake_request = {`
		`+ "objectType": "containerRepository",`
		`+ "projection": [`
		`+ {"fiel": "creationDate", "include": True},`
		`+ ],`
		`+ }`
		`+`
		`+ with patch('os.path.exists'):`
		`+ lb = LightBlue(server_url=self.fake_server_url,`
		`+ cert=self.fake_cert_file,`
		`+ private_key=self.fake_private_key)`
		`+ self.assertRaises(LightBlueRequestFailure,`
		`+ lb._make_request, 'find/containerRepository/', fake_request)`
		`+`
		`+`
		`+ class TestEntityVersion(unittest.TestCase):`
		`+ """Test case for ensuring correct entity version in request"""`
		`+`
		`+ def setUp(self):`
		`+ self.fake_server_url = 'lightblue.localhost'`
		`+ self.fake_cert_file = 'path/to/cert'`
		`+ self.fake_private_key = 'path/to/private-key'`
		`+ self.fake_entity_versions = {`
		`+ 'containerImage': '0.0.11'`
		`+ }`
		`+`
		`+ @patch('freshmaker.lightblue.LightBlue._make_request')`
		`+ @patch('os.path.exists')`
		`+ def test_use_default_entity_version(self, exists, _make_request):`
		`+ exists.return_value = True`
		`+`
		`+ 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)`
		`+ fake_request = {}`
		`+ lb.find_container_repositories({})`
		`+`
		`+ _make_request.assert_called_once_with('find/containerRepository/', {})`
		`+`
		`+ @patch('freshmaker.lightblue.LightBlue._make_request')`
		`+ @patch('os.path.exists')`
		`+ def test_use_specified_entity_version(self, exists, _make_request):`
		`+ exists.return_value = True`
		`+`
		`+ 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)`
		`+ fake_request = {}`
		`+ lb.find_container_images({})`
		`+`
		`+ _make_request.assert_called_once_with('find/containerImage/0.0.11', {})`
		`+`
		`+ @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`
		`+ _make_request.return_value = {`
		`+ # Omit other attributes that are not useful for this test`
		`+ 'processed': []`
		`+ }`
		`+`
		`+ lb = LightBlue(server_url=self.fake_server_url,`
		`+ cert=self.fake_cert_file,`
		`+ private_key=self.fake_private_key)`
		`+ fake_request = {}`
		`+ lb.find_container_repositories({})`
		`+ lb.find_container_images({})`
		`+`
		`+ _make_request.assert_has_calls([`
		`+ call('find/containerRepository/', {}),`
		`+ call('find/containerImage/', {}),`
		`+ ])`

cqi commented 8 years ago

A new class LightBlue to query entities containerImage and
containerRepository
New classes ContainerRepository and ContainerImage to represent the
repository and image objects
Make it possible to configure LightBlue

Signed-off-by: Chenxiong Qi cqi@redhat.com

qwan commented on line 81 of freshmaker/lightblue.py 8 years ago

According to the cases in tests, seems query is a complex data structure for an API to use, can we make some improvements on this? Or at least document it?

cqi commented on line 81 of freshmaker/lightblue.py 8 years ago

LightBlue documentation should be referenced to know how to write query. Document could be out-dated if the documentation gets updated in upstream.

This is a lower level API to query LightBlue. When using this API to implement docker image rebuild with LightBlue, there will be concrete query dict written and passed via this query parameter, and that could also be an example how the query looks like in practice.

I think I can add a short description of query and give the link of upstream documentation. That could be helpful hopefully. What do you think?

sochotni commented on line 26 of conf/config.py 8 years ago

Any reason to default to False really?

sochotni commented on line 84 of freshmaker/lightblue.py 8 years ago

I would suggest you add variables for schema versions to be used in these queries for containerRepository and containerImage. You use default schema version which means it's not pinned and could change for you.

sochotni commented on line 87 of freshmaker/lightblue.py 8 years ago

This will throw KeyError exception if there is any error message from LB - for example if the query is wrong.

sochotni commented on line 93 of freshmaker/lightblue.py 8 years ago

I would suggest making this and find_container_repositories into private methods and provide a public method with specific parameters for specific use cases. I.e something like

get_repositories_with_content_set(self, content_set)
get_images_in_repository(self, repository_id)
etc

sochotni commented on line 71 of freshmaker/lightblue.py 8 years ago

Is the query supposed to be full query including the projection, sort etc? If so - that's confusing use since you can see query is just one part of the request:
https://docs.lightblue.io/cookbook/find_data.html

jkaluza commented on line 81 of freshmaker/lightblue.py 8 years ago

I'm OK with cqi's explanation here. +1

jkaluza commented on line 84 of freshmaker/lightblue.py 8 years ago

jkaluza commented on line 93 of freshmaker/lightblue.py 8 years ago

According to cqi's comment on another part of PR, I think this is what he will do in the end. So far the task is to write a class to query lightblue. I think we can easily change the API here once we will find out high-level use-cases.

sochotni commented on line 93 of freshmaker/lightblue.py 8 years ago

Fair enough, but I'd still suggest we start with private methods (i.e. __find_container_images etc)

sochotni commented on line 65 of tests/test_lightblue.py 8 years ago

I'd recommend adding fail-tests (i.e. tests simulating request.post failure and errors from LB)

cqi commented on line 26 of conf/config.py 8 years ago

This option is for the test configuration. I thought in test environment to run freshmaker, disabling SSL verify could make things simple. I'm happy to change it to True as default, if we need to force to verify SSL in all environments.

jkaluza commented on line 26 of conf/config.py 8 years ago

I'm OK with having the dev and test configuration to not verify ssl. Just keep the BaseConfiguration set to True.

cqi commented on line 71 of freshmaker/lightblue.py 8 years ago

Yes. Query should be a full query in a POST request to LightBlue, and this class does not make GET request to LightBlue, instead, only POST.

Sorry, I don't get your point here, why do you think it might a confusing use?

cqi commented on line 93 of freshmaker/lightblue.py 8 years ago

Hi sochotni, as Jan mentioned above, we need to find out high-level use cases next. So far, it is not clear enough to know if this class is enough to include all code interacting with LightBlue, or need another place to include code to implement higher level features, for example, those methods you listed. As the first step, just keep this class as the very low level API, and make changes to it if necessary.

sochotni commented on line 71 of freshmaker/lightblue.py 8 years ago

OK, so an example of the query parameter will be following:
{
"objectType": "containerRepository",
"query": {
"field": "_id",
"regex": "."
},
"projection": [
{"field": "", "include": true, "recursive": true}

}

In other words - only one part of the query parameter is actually the lightblue query. Rest of it is other things.

sochotni commented on line 93 of freshmaker/lightblue.py 8 years ago

I realize - it was just a suggestion. Having a very generic method be part of your class API means that you can't control error handling very well. Using private method designator (__ prefix) doesn't prevent you from working on higher level features later. It just means "hey, don't use this method directly from outside" to the users of your class. It can still be used by subclasses easily.

cqi commented on line 93 of freshmaker/lightblue.py 8 years ago

Hi sochotni, cool, get your point. You are considering from the view of inheritance, whereas I was thinking of it from view of composition only :) Your idea makes sense to me.

cqi commented on line 71 of freshmaker/lightblue.py 8 years ago

Hi sochotni,

I see. The reason of why I use the full query is to allow the ability of selecting part of fields. Because not the whole image or repository data is required always, for example, maybe _id and brew are just required for starting rebuild of docker image.

To avoid confusion, how about make query, fields (the projection), range, and sort as separate parameters? Is it worth to do?

sochotni commented on line 71 of freshmaker/lightblue.py 8 years ago

That could be one way to do things. Simpler approach: just rename the parameter to "request"? :-)

sochotni commented on line 49 of freshmaker/lightblue.py 8 years ago

We really should create a separate python library for interacting with Lightblue that we could all use...

rebased

8 years ago

jkaluza commented 8 years ago

Is it ready for another review?

cqi commented 8 years ago

Is it ready for another review?

@jkaluza yes

cqi commented 8 years ago

We really should create a separate python library for interacting with Lightblue that we could all use...

@sochotni I agree indeed. I searched before, so far, haven't found such a Python library. We could do it :)

cqi commented 8 years ago

Seems my previous comment describing the updated patch includes is lost :( I can't find it in this PR. Now, let me recover it

updates

handle error returned from LightBlue
add tests for the error handler
rename parameters' name in method find_container_* and _make_request to avoid potential confusion, and add description for parameters
specify version in URL to request data
fix _make_request. Content-Type header is required in request