#787 Make our builders into setuptools plugins.
Merged a year ago by mprahl. Opened a year ago by ralph.

@@ -1,17 +1,10 @@ 

- from module_build_service import conf

+ import pkg_resources

+ 

  from base import GenericBuilder

- from KojiModuleBuilder import KojiModuleBuilder

- from MockModuleBuilder import MockModuleBuilder

  

  __all__ = [

      GenericBuilder

  ]

  

- 

- GenericBuilder.register_backend_class(KojiModuleBuilder)

- 

- GenericBuilder.register_backend_class(MockModuleBuilder)

- 

- if conf.system == "copr":

-     from CoprModuleBuilder import CoprModuleBuilder

-     GenericBuilder.register_backend_class(CoprModuleBuilder)

+ for entrypoint in pkg_resources.iter_entry_points('mbs.builder_backends'):

+     GenericBuilder.register_backend_class(entrypoint.load())

@@ -65,7 +65,7 @@ 

  

          if conf.system == 'koji':

              # We don't do this on behalf of users

-             koji_session = module_build_service.builder.KojiModuleBuilder\

+             koji_session = module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder\

                  .get_session(conf, None)

              log.info('Querying tasks for statuses:')

              res = models.ComponentBuild.query.filter_by(

@@ -231,7 +231,7 @@ 

          if config.system != 'koji':

              return

  

-         koji_session = module_build_service.builder.KojiModuleBuilder\

+         koji_session = module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder\

              .get_session(config, None)

  

          for module_build in session.query(models.ModuleBuild) \

@@ -263,7 +263,7 @@ 

  

          now = datetime.utcnow()

  

-         koji_session = module_build_service.builder.KojiModuleBuilder\

+         koji_session = module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder\

              .get_session(config, None)

          for target in koji_session.getBuildTargets():

              koji_tag = target["dest_tag_name"]

file modified
+8 -1

@@ -33,7 +33,14 @@ 

                'fedmsg = module_build_service.messaging:_fedmsg_backend',

                'in_memory = module_build_service.messaging:_in_memory_backend',

                # 'custom = your_organization:_custom_backend',

-           ]

+           ],

+           'mbs.builder_backends': [

+               'koji = module_build_service.builder.KojiModuleBuilder:KojiModuleBuilder',

+               'mock = module_build_service.builder.MockModuleBuilder:MockModuleBuilder',

+               # TODO - let's move this out into its own repo so @frostyx can

+               # iterate without us blocking him.

+               'copr = module_build_service.builder.CoprModuleBuilder:CoprModuleBuilder',

+           ],

        },

        scripts=["contrib/mbs-build"],

        data_files=[('/etc/module-build-service/', ['conf/cacert.pem',

@@ -43,7 +43,8 @@ 

  import json

  import itertools

  

- from module_build_service.builder import KojiModuleBuilder, GenericBuilder

+ from module_build_service.builder.base import GenericBuilder

+ from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder

  import module_build_service.scheduler.consumer

  from module_build_service.messaging import MBSModule

  

@@ -34,7 +34,7 @@ 

  

  from tests import conf, init_data

  

- from module_build_service.builder import KojiModuleBuilder

+ from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder

  

  

  class FakeKojiModuleBuilder(KojiModuleBuilder):

@@ -9,7 +9,7 @@ 

  

  from module_build_service import conf

  from module_build_service.models import ModuleBuild, ComponentBuild, make_session

- from module_build_service.builder import MockModuleBuilder

+ from module_build_service.builder.MockModuleBuilder import MockModuleBuilder

  from tests import db, init_data

  

  

@@ -162,7 +162,7 @@ 

          with open(path.join(dir_path, "modulemd.txt")) as mmd:

              self.assertEqual(len(mmd.read()), 1134)

  

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      def test_tag_cg_build(self, get_session):

          """ Test that the CG build is tagged. """

          koji_session = MagicMock()

@@ -174,7 +174,7 @@ 

          koji_session.getTag.assert_called_once_with(self.cg.module.cg_build_koji_tag)

          koji_session.tagBuild.assert_called_once_with(123, "nginx-1-2")

  

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      def test_tag_cg_build_fallback_to_default_tag(self, get_session):

          """ Test that the CG build is tagged to default tag. """

          koji_session = MagicMock()

@@ -188,7 +188,7 @@ 

                            call(conf.koji_cg_default_build_tag)])

          koji_session.tagBuild.assert_called_once_with(123, "nginx-1-2")

  

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      def test_tag_cg_build_no_tag_set(self, get_session):

          """ Test that the CG build is not tagged when no tag set. """

          koji_session = MagicMock()

@@ -200,7 +200,7 @@ 

  

          koji_session.tagBuild.assert_not_called()

  

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      def test_tag_cg_build_no_tag_available(self, get_session):

          """ Test that the CG build is not tagged when no tag available. """

          koji_session = MagicMock()

@@ -93,7 +93,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      @patch('module_build_service.pdc')

      def test_new_repo_called_when_macros_reused(

@@ -130,7 +130,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      @patch('module_build_service.pdc')

      def test_new_repo_not_called_when_macros_not_reused(

@@ -161,7 +161,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      @patch('module_build_service.pdc')

      def test_set_cg_build_koji_tag_fallback_to_default(

@@ -199,7 +199,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      @patch('module_build_service.pdc')

      @patch("module_build_service.config.Config.base_module_names",

@@ -37,7 +37,7 @@ 

  @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

         return_value={'build': [], 'srpm-build': []})

  @patch("module_build_service.scheduler.consumer.get_global_consumer")

- @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+ @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

  @patch("module_build_service.builder.GenericBuilder.create_from_module")

  class TestPoller(unittest.TestCase):

  

@@ -58,14 +58,20 @@ 

          module_build_service.scheduler.handlers.repos.done(

              config=conf, session=db.session, msg=msg)

  

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.get_average_build_time',

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.get_average_build_time',

                  return_value=0.0)

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.list_tasks_for_components',

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.list_tasks_for_components',

                  return_value=[])

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.buildroot_ready', return_value=True)

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.get_session')

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.build')

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.buildroot_connect')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.buildroot_ready', return_value=True)

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.get_session')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.build')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.buildroot_connect')

      def test_a_single_match(self, connect, build_fn, get_session, ready, list_tasks_fn, mock_gabt):

          """ Test that when a repo msg hits us and we have a single match.

          """

@@ -81,14 +87,20 @@ 

              source=('git://pkgs.domain.local/rpms/communicator'

                      '?#da95886c8a443b36a9ce31abda1f9bed22f2f9c2'))

  

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.get_average_build_time',

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.get_average_build_time',

                  return_value=0.0)

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.list_tasks_for_components',

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.list_tasks_for_components',

                  return_value=[])

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.buildroot_ready', return_value=True)

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.get_session')

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.build')

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.buildroot_connect')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.buildroot_ready', return_value=True)

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.get_session')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.build')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.buildroot_connect')

      def test_a_single_match_build_fail(self, connect, build_fn, config, ready, list_tasks_fn,

                                         mock_gabt):

          """ Test that when a KojiModuleBuilder.build fails, the build is

@@ -131,12 +143,17 @@ 

          # Make sure the module build didn't transition since all the components weren't tagged

          self.assertEqual(module_build.state, module_build_service.models.BUILD_STATES['build'])

  

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.list_tasks_for_components',

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.list_tasks_for_components',

                  return_value=[])

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.buildroot_ready', return_value=True)

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.get_session')

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.build')

-     @mock.patch('module_build_service.builder.KojiModuleBuilder.buildroot_connect')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.buildroot_ready', return_value=True)

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.get_session')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.build')

+     @mock.patch('module_build_service.builder.KojiModuleBuilder.'

+                 'KojiModuleBuilder.buildroot_connect')

      @mock.patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

                  return_value={'build': [], 'srpm-build': []})

      def test_failed_component_build(self, dbg, connect, build_fn, config, ready, list_tasks_fn):

@@ -64,7 +64,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      def test_newrepo(self, create_builder, koji_get_session, dbg):

          """

@@ -140,7 +140,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      def test_newrepo_still_building_components(self, create_builder, koji_get_session, dbg):

          """

@@ -182,7 +182,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      def test_newrepo_failed_components(self, create_builder, koji_get_session, dbg):

          """

@@ -243,7 +243,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      def test_newrepo_multiple_batches_tagged(

              self, create_builder, koji_get_session, dbg):

@@ -328,7 +328,7 @@ 

  

      @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups",

             return_value={'build': [], 'srpm-build': []})

-     @patch("module_build_service.builder.KojiModuleBuilder.get_session")

+     @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")

      @patch("module_build_service.builder.GenericBuilder.create_from_module")

      def test_newrepo_build_time_only(

              self, create_builder, koji_get_session, dbg):

@@ -34,7 +34,8 @@ 

  import mock

  import koji

  import module_build_service.scheduler.handlers.components

- from module_build_service.builder import GenericBuilder, KojiModuleBuilder

+ from module_build_service.builder.base import GenericBuilder

+ from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder

  from tests import app

  

  BASE_DIR = path.abspath(path.dirname(__file__))

@frostyx, can you take a look here?

The thought is that we can use this as a stepping stone to giving you more
control over the CoprModuleBuilder. If this works out, we can move
CoprModuleBuilder out to its own repo/project where you and the team can have
full control over what gets merged and when it gets released.

Hi @ralph, thank you for bringing this topic up.

I am kinda happy with the current state, cause modularity/factory guys give me a feedback when I am trying to do something stupid, but I would also appreciate having more control over Copr-related stuff, so +1 for this. I have some concerns though.

  • With #738 in mind, we shouldn't talk about separating builders to it's own repos, but rather about something more general. There could be custom Builders, Resolvers and possibly more.
  • RPM packaging - I suppose that I would have to create my own RPM package for Copr plugin for MBS? Or maybe we could create something like module-build-service-plugins with subpackages for every build system (like DNF has)? It would probably depend on whether you want to have all custom builders/plugins in one repo or one per repo.
  • Now if there is a change regarding builders, someone will probably update all of them, but once they are in another repo, it is easier to forget them and leave them untouched. It would be nice if we could stabilize the builder API first.

But yes, defining builder backends as entrypoints seems to be a good first step which shouldn't cause any harm regardless what I wrote in the previous post, so +1 for this PR from me.

I think there is one too many KojiModuleBuilder in this statement. As I understand it, it should be:

'koji = module_build_service.builder.KojiModuleBuilder:KojiModuleBuilder',

True, true.

2 new commits added

  • Make our builders into setuptools plugins.
  • Explicit imports of builders in the test suite.
a year ago

2 new commits added

  • Make our builders into setuptools plugins.
  • Explicit imports of builders in the test suite.
a year ago

OK, flake8 passes now, but the test suite appears to be hanging.

I am kinda happy with the current state, cause modularity/factory guys give me a feedback when I am trying to do something stupid, but I would also appreciate having more control over Copr-related stuff, so +1 for this. I have some concerns though.

Makes sense, yeah.

With #738 in mind, we shouldn't talk about separating builders to it's own repos, but rather about something more general. There could be custom Builders, Resolvers and possibly more.

Agreed. Let's take that on in a second or third PR.

RPM packaging - I suppose that I would have to create my own RPM package for Copr plugin for MBS? Or maybe we could create something like module-build-service-plugins with subpackages for every build system (like DNF has)? It would probably depend on whether you want to have all custom builders/plugins in one repo or one per repo.

Unfortunately, yes. I'd recommend separate packaging for different plugins - just to make constructing updates simpler.

Now if there is a change regarding builders, someone will probably update all of them, but once they are in another repo, it is easier to forget them and leave them untouched. It would be nice if we could stabilize the builder API first.

Yeah, we'll have to be more careful with this going forwards.

Commit c33a22f fixes this pull-request

Pull-Request has been merged by mprahl@redhat.com

a year ago

Pull-Request has been merged by mprahl

a year ago