diff --git a/module_build_service/builder/CoprModuleBuilder.py b/module_build_service/builder/CoprModuleBuilder.py index e12d1670..68b34726 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) @@ -93,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: @@ -116,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, @@ -254,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, 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..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,9 +61,11 @@ 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 + 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, @@ -82,8 +84,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='nginx', + fake_kmb = FakeKojiModuleBuilder(owner=self.module.owner, + module=self.module, config=conf, tag_name='module-nginx-1.2', components=[]) @@ -98,8 +100,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='nginx', + builder = FakeKojiModuleBuilder(owner=self.module.owner, + module=self.module, config=conf, tag_name='module-nginx-1.2', components=[]) @@ -130,20 +132,20 @@ 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 - self.owner = 'Matt Jia' - self.module = '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())))