fm-orchestrator

Clone
Source Code
GIT

#787 Make our builders into setuptools plugins.
Merged 6 years ago by mprahl. Opened 6 years ago by ralph.
builders-as-plugins  into  master

file modified
+4 -11 
@@ -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())

file modified
+3 -3 
@@ -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',

file modified
+2 -1 
@@ -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

file modified
+1 -1 
@@ -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):

file modified
+1 -1 
@@ -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

file modified
+4 -4 
@@ -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()

file modified
+4 -4 
@@ -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",

file modified
+1 -1 
@@ -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):

file modified
+34 -17 
@@ -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):

file modified
+5 -5 
@@ -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):

file modified
+2 -1 
@@ -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.
6 years ago

2 new commits added

  • Make our builders into setuptools plugins.
  • Explicit imports of builders in the test suite.
6 years 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.

+1

Commit c33a22f fixes this pull-request

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

Pull-Request has been merged by mprahl
6 years ago
Metadata
None
No Tags
Changes Summary 12
+4 -11
file changed
module_build_service/builder/__init__.py
+3 -3
file changed
module_build_service/scheduler/producer.py
+8 -1
file changed
setup.py
+2 -1
file changed
tests/test_build/test_build.py
+1 -1
file changed
tests/test_builder/test_koji.py
+1 -1
file changed
tests/test_builder/test_mock.py
+4 -4
file changed
tests/test_content_generator.py
+4 -4
file changed
tests/test_scheduler/test_module_wait.py
+1 -1
file changed
tests/test_scheduler/test_poller.py
+34 -17
file changed
tests/test_scheduler/test_repo_done.py
+5 -5
file changed
tests/test_scheduler/test_tag_tagged.py
+2 -1
file changed
tests/test_utils/test_utils.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.