From b78b0346d2bea2ed8f9918d591debd8c524cc265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Kadl=C4=8D=C3=ADk?= Date: Wed, 12 Apr 2017 22:52:45 +0200 Subject: [PATCH 1/4] Pass the module build into builder --- module_build_service/builder/CoprModuleBuilder.py | 3 ++- module_build_service/builder/KojiModuleBuilder.py | 4 ++-- module_build_service/builder/MockModuleBuilder.py | 2 +- module_build_service/builder/base.py | 4 ++-- module_build_service/scheduler/handlers/modules.py | 2 +- module_build_service/scheduler/handlers/repos.py | 2 +- tests/test_builder/test_koji.py | 11 ++++++++--- 7 files changed, 17 insertions(+), 11 deletions(-) diff --git a/module_build_service/builder/CoprModuleBuilder.py b/module_build_service/builder/CoprModuleBuilder.py index e12d1670..ad9703bb 100644 --- a/module_build_service/builder/CoprModuleBuilder.py +++ b/module_build_service/builder/CoprModuleBuilder.py @@ -60,7 +60,8 @@ class CoprModuleBuilder(GenericBuilder): self.owner = owner self.config = config self.tag_name = tag_name - self.module_str = module + self.module = module + self.module_str = module.name self.copr = None self.client = CoprModuleBuilder._get_client(config) diff --git a/module_build_service/builder/KojiModuleBuilder.py b/module_build_service/builder/KojiModuleBuilder.py index 8ff4ea81..8350995e 100644 --- a/module_build_service/builder/KojiModuleBuilder.py +++ b/module_build_service/builder/KojiModuleBuilder.py @@ -61,12 +61,12 @@ class KojiModuleBuilder(GenericBuilder): def __init__(self, owner, module, config, tag_name, components): """ :param owner: a string representing who kicked off the builds - :param module: string representing module + :param module: module_build_service.models.ModuleBuild instance. :param config: module_build_service.config.Config instance :param tag_name: name of tag for given module """ self.owner = owner - self.module_str = module + self.module_str = module.name self.config = config self.tag_name = tag_name self.__prep = False diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index 57191982..a4b6a724 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -94,7 +94,7 @@ mdpolicy=group:primary @module_build_service.utils.validate_koji_tag('tag_name') def __init__(self, owner, module, config, tag_name, components): - self.module_str = module + self.module_str = module.name self.tag_name = tag_name self.config = config self.groups = [] diff --git a/module_build_service/builder/base.py b/module_build_service/builder/base.py index 4407ba93..51286198 100644 --- a/module_build_service/builder/base.py +++ b/module_build_service/builder/base.py @@ -100,7 +100,7 @@ class GenericBuilder(six.with_metaclass(ABCMeta)): def create(cls, owner, module, backend, config, **extra): """ :param owner: a string representing who kicked off the builds - :param module: a module string e.g. 'testmodule-1.0' + :param module: module_build_service.models.ModuleBuild instance. :param backend: a string representing backend e.g. 'koji' :param config: instance of module_build_service.config.Config @@ -125,7 +125,7 @@ class GenericBuilder(six.with_metaclass(ABCMeta)): """ components = [c.package for c in module.component_builds] builder = GenericBuilder.create( - module.owner, module.name, config.system, config, + module.owner, module, config.system, config, tag_name=module.koji_tag, components=components) groups = GenericBuilder.default_buildroot_groups(session, module) builder.buildroot_connect(groups) diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index a485fb5f..ce01d986 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -81,7 +81,7 @@ def failed(config, session, msg): if build.koji_tag: builder = module_build_service.builder.GenericBuilder.create( - build.owner, build.name, config.system, config, tag_name=build.koji_tag, + build.owner, build, config.system, config, tag_name=build.koji_tag, components=[c.package for c in build.component_builds]) builder.buildroot_connect(groups) diff --git a/module_build_service/scheduler/handlers/repos.py b/module_build_service/scheduler/handlers/repos.py index 5f99c966..8090423c 100644 --- a/module_build_service/scheduler/handlers/repos.py +++ b/module_build_service/scheduler/handlers/repos.py @@ -85,7 +85,7 @@ def done(config, session, msg): session, module_build) builder = module_build_service.builder.GenericBuilder.create( - module_build.owner, module_build.name, config.system, config, + module_build.owner, module_build, config.system, config, tag_name=tag, components=[c.package for c in module_build.component_builds]) builder.buildroot_connect(groups) diff --git a/tests/test_builder/test_koji.py b/tests/test_builder/test_koji.py index fc58143e..2a9f8657 100644 --- a/tests/test_builder/test_koji.py +++ b/tests/test_builder/test_koji.py @@ -25,6 +25,7 @@ import munch import mock import koji import xmlrpclib +from collections import namedtuple import module_build_service.messaging import module_build_service.scheduler.handlers.repos @@ -58,6 +59,10 @@ class FakeKojiModuleBuilder(KojiModuleBuilder): return koji_session + +ModuleBuildMock = namedtuple('ModuleBuildMock', ['name']) + + class TestKojiBuilder(unittest.TestCase): def setUp(self): @@ -83,7 +88,7 @@ class TestKojiBuilder(unittest.TestCase): 'checkForBuilds.side_effect': IOError} mocked_kojiutil.configure_mock(**attrs) fake_kmb = FakeKojiModuleBuilder(owner='Moe Szyslak', - module='nginx', + module=ModuleBuildMock(name='nginx'), config=conf, tag_name='module-nginx-1.2', components=[]) @@ -99,7 +104,7 @@ class TestKojiBuilder(unittest.TestCase): tag already tagged artifacts """ builder = FakeKojiModuleBuilder(owner='Moe Szyslak', - module='nginx', + module=ModuleBuildMock(name='nginx'), config=conf, tag_name='module-nginx-1.2', components=[]) @@ -134,7 +139,7 @@ class TestGetKojiClientSession(unittest.TestCase): self.config.koji_profile = conf.koji_profile self.config.koji_config = conf.koji_config self.owner = 'Matt Jia' - self.module = 'fool' + self.module = ModuleBuildMock(name='fool') self.tag_name = 'module-fool-1.2' @patch.object(koji.ClientSession, 'krb_login') From a64a57cd9a9ce318dd0525735ce04768128b01f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Kadl=C4=8D=C3=ADk?= Date: Wed, 12 Apr 2017 23:15:47 +0200 Subject: [PATCH 2/4] Don't query module which may be incorrect --- .../builder/CoprModuleBuilder.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/module_build_service/builder/CoprModuleBuilder.py b/module_build_service/builder/CoprModuleBuilder.py index ad9703bb..68b34726 100644 --- a/module_build_service/builder/CoprModuleBuilder.py +++ b/module_build_service/builder/CoprModuleBuilder.py @@ -94,12 +94,9 @@ class CoprModuleBuilder(GenericBuilder): log.info("%r buildroot sucessfully connected." % self) def _get_copr_safe(self): - # @TODO it would be nice if the module build object was passed to Builder __init__ - module = ModuleBuild.query.filter(ModuleBuild.name == self.module_str).one() - kwargs = { - "ownername": module.copr_owner or self.owner, - "projectname": module.copr_project or CoprModuleBuilder._tag_to_copr_name(self.tag_name) + "ownername": self.module.copr_owner or self.owner, + "projectname": self.module.copr_project or CoprModuleBuilder._tag_to_copr_name(self.tag_name) } try: @@ -117,14 +114,12 @@ class CoprModuleBuilder(GenericBuilder): def _create_module_safe(self): from copr.exceptions import CoprRequestException - # @TODO it would be nice if the module build object was passed to Builder __init__ - module = ModuleBuild.query.filter(ModuleBuild.name == self.module_str).one() modulemd = tempfile.mktemp() - module.mmd().dump(modulemd) + self.module.mmd().dump(modulemd) kwargs = { - "username": module.copr_owner or self.owner, - "projectname": module.copr_project or CoprModuleBuilder._tag_to_copr_name(self.tag_name), + "username": self.module.copr_owner or self.owner, + "projectname": self.module.copr_project or CoprModuleBuilder._tag_to_copr_name(self.tag_name), "modulemd": modulemd, "create": True, "build": False, @@ -255,8 +250,7 @@ class CoprModuleBuilder(GenericBuilder): def finalize(self): modulemd = tempfile.mktemp() - m1 = ModuleBuild.query.filter(ModuleBuild.name == self.module_str).one() - m1.mmd().dump(modulemd) + self.module.mmd().dump(modulemd) # Create a module from previous project result = self.client.make_module(username=self.copr.username, projectname=self.copr.projectname, From e3c3167fae237c3c140e7036793869888c9ee2bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Kadl=C4=8D=C3=ADk?= Date: Thu, 13 Apr 2017 16:44:55 +0200 Subject: [PATCH 3/4] Don't mock the ModuleBuild class with namedtuple for tests --- tests/test_builder/test_koji.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/tests/test_builder/test_koji.py b/tests/test_builder/test_koji.py index 2a9f8657..a872c573 100644 --- a/tests/test_builder/test_koji.py +++ b/tests/test_builder/test_koji.py @@ -25,7 +25,6 @@ import munch import mock import koji import xmlrpclib -from collections import namedtuple import module_build_service.messaging import module_build_service.scheduler.handlers.repos @@ -59,16 +58,13 @@ class FakeKojiModuleBuilder(KojiModuleBuilder): return koji_session - -ModuleBuildMock = namedtuple('ModuleBuildMock', ['name']) - - class TestKojiBuilder(unittest.TestCase): def setUp(self): self.config = mock.Mock() self.config.koji_profile = conf.koji_profile self.config.koji_repository_url = conf.koji_repository_url + self.module = module_build_service.models.ModuleBuild.query.filter_by(id=1).one() def test_tag_to_repo(self): """ Test that when a repo msg hits us and we have no match, @@ -87,8 +83,8 @@ class TestKojiBuilder(unittest.TestCase): attrs = {'checkForBuilds.return_value': None, 'checkForBuilds.side_effect': IOError} mocked_kojiutil.configure_mock(**attrs) - fake_kmb = FakeKojiModuleBuilder(owner='Moe Szyslak', - module=ModuleBuildMock(name='nginx'), + fake_kmb = FakeKojiModuleBuilder(owner=self.module.owner, + module=self.module, config=conf, tag_name='module-nginx-1.2', components=[]) @@ -103,8 +99,8 @@ class TestKojiBuilder(unittest.TestCase): Tests that buildroot_add_artifacts and tag_artifacts do not try to tag already tagged artifacts """ - builder = FakeKojiModuleBuilder(owner='Moe Szyslak', - module=ModuleBuildMock(name='nginx'), + builder = FakeKojiModuleBuilder(owner=self.module.owner, + module=self.module, config=conf, tag_name='module-nginx-1.2', components=[]) @@ -138,17 +134,16 @@ class TestGetKojiClientSession(unittest.TestCase): self.config = mock.Mock() self.config.koji_profile = conf.koji_profile self.config.koji_config = conf.koji_config - self.owner = 'Matt Jia' - self.module = ModuleBuildMock(name='fool') + self.module = module_build_service.models.ModuleBuild.query.filter_by(id=1).one() self.tag_name = 'module-fool-1.2' @patch.object(koji.ClientSession, 'krb_login') def test_proxyuser(self, mocked_krb_login): - KojiModuleBuilder(owner=self.owner, + KojiModuleBuilder(owner=self.module.owner, module=self.module, config=self.config, tag_name=self.tag_name, components=[]) args, kwargs = mocked_krb_login.call_args - self.assertTrue(set([('proxyuser', self.owner)]).issubset(set(kwargs.items()))) + self.assertTrue(set([('proxyuser', self.module.owner)]).issubset(set(kwargs.items()))) From e8dd847cea70e65311bc9de06e9106046ac86eb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Kadl=C4=8D=C3=ADk?= Date: Thu, 13 Apr 2017 17:42:12 +0200 Subject: [PATCH 4/4] Run init_data() before querying a module --- tests/test_builder/test_koji.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_builder/test_koji.py b/tests/test_builder/test_koji.py index a872c573..7a370a7e 100644 --- a/tests/test_builder/test_koji.py +++ b/tests/test_builder/test_koji.py @@ -33,7 +33,7 @@ import module_build_service.builder from mock import patch, MagicMock -from tests import conf +from tests import conf, init_data from module_build_service.builder import KojiModuleBuilder @@ -61,6 +61,7 @@ class FakeKojiModuleBuilder(KojiModuleBuilder): class TestKojiBuilder(unittest.TestCase): def setUp(self): + init_data() self.config = mock.Mock() self.config.koji_profile = conf.koji_profile self.config.koji_repository_url = conf.koji_repository_url @@ -131,6 +132,7 @@ class TestKojiBuilder(unittest.TestCase): class TestGetKojiClientSession(unittest.TestCase): def setUp(self): + init_data() self.config = mock.Mock() self.config.koji_profile = conf.koji_profile self.config.koji_config = conf.koji_config