From 6e4429587e353b2aa2d6204d6d60ce58044af026 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Mar 19 2018 13:18:52 +0000 Subject: Make PDC.get_latest_modules faster Fixes #218 Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/pdc.py b/freshmaker/pdc.py index 306e19b..7fff388 100644 --- a/freshmaker/pdc.py +++ b/freshmaker/pdc.py @@ -22,8 +22,9 @@ import inspect import requests +import concurrent.futures +from itertools import groupby from pdc_client import PDCClient - import freshmaker.utils @@ -52,22 +53,36 @@ class PDC(object): insecure=self.config.pdc_insecure, ) - def get_latest_modules(self, **kwargs): - """ - Query PDC with query parameters in kwargs and return a list of modules - which contains latest modules of each (module_name, module_version). + def is_latest_module(self, module): + """Check if given module is the latest one in the name:stream in PDC""" + data = self.get_modules( + name=module['name'], + stream=module['stream'], + fields='version', + ordering='-version', + page_size=1) + return data['results'][0]['version'] == module['version'] - :param kwargs: query parameters in keyword arguments, should only provide - valid query parameters supported by PDC's module query API. - :return: a list of modules - """ - modules = self.get_modules(**kwargs) - active = kwargs.get('active', 'true') - latest_modules = [] - for (name, stream) in set([(m.get('name'), m.get('stream')) for m in modules]): - mods = self.get_modules(name=name, stream=stream, active=active) - latest_modules.append(sorted(mods, key=lambda x: x['version']).pop()) - return list(filter(lambda x: x in latest_modules, modules)) + def get_latest_modules(self, **criteria): + criteria.update({ + 'fields': ['uid', 'name', 'stream', 'version'], + 'ordering': 'name,stream,-version', + }) + modules = self.get_modules(**criteria) + + def _return_module_if_latest(module): + return module if self.is_latest_module(module) else None + + with concurrent.futures.ThreadPoolExecutor(max_workers=10) as executor: + futures = [ + executor.submit(_return_module_if_latest, + list(stream_modules)[0]) + for name_stream, stream_modules in groupby( + modules, key=lambda m: '%(name)s:%(stream)s' % m) + ] + concurrent.futures.wait(futures) + + return [m for m in (f.result() for f in futures) if m] @freshmaker.utils.retry(wait_on=(requests.Timeout, requests.ConnectionError), logger=freshmaker.log) def get_modules(self, **kwargs): @@ -77,7 +92,8 @@ class PDC(object): :param kwargs: query parameters in keyword arguments :return: a list of modules """ - modules = self.session['modules'](page_size=-1, **kwargs) + page_size = kwargs.pop('page_size', -1) + modules = self.session['modules'](page_size=page_size, **kwargs) return modules @freshmaker.utils.retry(wait_on=(requests.Timeout, requests.ConnectionError), logger=freshmaker.log) diff --git a/tests/test_pdc.py b/tests/test_pdc.py index a49c520..adae26d 100644 --- a/tests/test_pdc.py +++ b/tests/test_pdc.py @@ -19,11 +19,9 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. - import unittest -from mock import call, patch - +from mock import patch from freshmaker import conf from freshmaker.pdc import PDC @@ -31,69 +29,79 @@ from freshmaker.pdc import PDC class TestGetLatestModules(unittest.TestCase): """Test PDC.get_latest_modules""" - @patch('freshmaker.pdc.PDC.get_modules') - def test_exclude_modules_that_doesnt_depend_on_built_module(self, get_modules): - get_modules.side_effect = [ - # modules returned from first call - [{'name': '389-ds', 'stream': '1.2', 'version': '20171009091843'}, - {'name': '389-ds', 'stream': '1.2', 'version': '20171012150041'}, - {'name': 'apache-commons', 'stream': 'f27', 'version': '20171010111836'}], - - # modules returned from call for name 386-ds and stream 1.2 - [{'name': '389-ds', 'stream': '1.2', 'version': '20171009105405'}, - {'name': '389-ds', 'stream': '1.2', 'version': '20171012150041'}, - - # *** This is a new version module that already depends on other module. - {'name': '389-ds', 'stream': '1.2', 'version': '20171120124934'}], - - # modules returned from call for name apache-commons and stream f27 - [{'name': 'apache-commons', 'stream': 'f27', 'version': '20171010111836'}] - ] - - pdc = PDC(conf) - modules = pdc.get_latest_modules(build_dep_name='rebuilt module', - build_dep_stream='1.7', - active=True) - - expected_modules = [ - {'name': 'apache-commons', 'stream': 'f27', 'version': '20171010111836'}, - ] - self.assertEqual(expected_modules, modules) + def mock_is_latest_module(self, module): + fake_results = { + '389-ds': True, + 'apache-commons': True, + 'nodejs': False + } + return fake_results[module['name']] @patch('freshmaker.pdc.PDC.get_modules') - def test_found_latest_modules(self, get_modules): + @patch('freshmaker.pdc.PDC.is_latest_module') + def test_get_latest_modules(self, is_latest_module, get_modules): + is_latest_module.side_effect = self.mock_is_latest_module get_modules.side_effect = [ - # modules returned from first call - [{'name': '389-ds', 'stream': '1.2', 'version': '20171009091843'}, - {'name': '389-ds', 'stream': '1.2', 'version': '20171012150041'}, - {'name': '389-ds', 'stream': '1.2', 'version': '20171120124934'}, - {'name': 'apache-commons', 'stream': 'f27', 'version': '20171010111836'}], - - # modules returned from call for name 386-ds and stream 1.2 - [{'name': '389-ds', 'stream': '1.2', 'version': '20171009105405'}, - {'name': '389-ds', 'stream': '1.2', 'version': '20171012150041'}, - {'name': '389-ds', 'stream': '1.2', 'version': '20171120124934'}], - - # modules returned from call for name apache-commons and stream f27 - [{'name': 'apache-commons', 'stream': 'f27', 'version': '20171010111836'}] + [ + { + 'uid': '389-ds-1.2-20171120124934', + 'name': '389-ds', + 'stream': '1.2', + 'version': '20171120124934' + }, + { + 'uid': '389-ds-1.2-20171012150041', + 'name': '389-ds', + 'stream': '1.2', + 'version': '20171012150041' + }, + { + 'uid': '389-ds-1.2-20171009091843', + 'name': '389-ds', + 'stream': '1.2', + 'version': '20171009091843' + }, + { + 'uid': 'apache-commons-f27-20171010111836', + 'name': 'apache-commons', + 'stream': 'f27', + 'version': '20171010111836' + }, + { + 'uid': 'nodejs:9:20180213214624:c2c572ec', + 'name': 'nodejs', + 'stream': '9', + 'version': '20180213214624' + }, + { + 'uid': 'nodejs-9-20180205182158', + 'name': 'nodejs', + 'stream': '9', + 'version': '20180205182158' + }, + ], ] pdc = PDC(conf) modules = pdc.get_latest_modules(build_dep_name='rebuilt module', build_dep_stream='1.7', active=True) - modules = sorted(modules, key=lambda m: m['name']) + + # Module nodejs should not be included because the its fake data aims + # to test the nodejs:9:20180213214624:c2c572ec is not latest module. expected_modules = [ - {'name': '389-ds', 'stream': '1.2', 'version': '20171120124934'}, - {'name': 'apache-commons', 'stream': 'f27', 'version': '20171010111836'}, + { + 'uid': '389-ds-1.2-20171120124934', + 'name': '389-ds', + 'stream': '1.2', + 'version': '20171120124934' + }, + { + 'uid': 'apache-commons-f27-20171010111836', + 'name': 'apache-commons', + 'stream': 'f27', + 'version': '20171010111836' + }, ] self.assertEqual(expected_modules, modules) - - get_modules.assert_has_calls([ - call(build_dep_name='rebuilt module', - build_dep_stream='1.7', - active=True), - call(name='389-ds', stream='1.2', active=True), - call(name='apache-commons', stream='f27', active=True), - ], any_order=True)