#1544 Remove unused handler arguments
Merged 4 years ago by qwan. Opened 4 years ago by qwan.
qwan/fm-orchestrator remove-unused-handler-args  into  v3

@@ -747,7 +747,7 @@ 

          nvr_dict = kobo.rpmlib.parse_nvr(component_build.nvr)

          # Trigger a completed build message

          args = (

-             "recover_orphaned_artifact: fake message", build["build_id"], build["task_id"],

+             "recover_orphaned_artifact: fake message", build["task_id"],

              koji.BUILD_STATES["COMPLETE"], component_build.package, nvr_dict["version"],

              nvr_dict["release"], component_build.module_build.id, None)

          events.scheduler.add(build_task_finalize_handler, args)
@@ -769,8 +769,7 @@ 

                  'The build being skipped isn\'t tagged in the "{0}" tag. Will send a message to '

                  "the tag handler".format(tag)

              )

-             args = ("recover_orphaned_artifact: fake message", tag, component_build.package,

-                     component_build.nvr)

+             args = ("recover_orphaned_artifact: fake message", tag, component_build.nvr)

              events.scheduler.add(tagged_handler, args)

  

          return True

@@ -399,10 +399,9 @@ 

          except ValueError:

              nvr = {"name": source, "release": "unknown", "version": "unknown"}

  

-         # build_id=1 and task_id=1 are OK here, because we are building just

-         # one RPM at the time.

+         # use build_id as task_id

          args = (

-             "a faked internal message", build_id, build_id, state, nvr["name"], nvr["version"],

+             "a faked internal message", build_id, state, nvr["name"], nvr["version"],

              nvr["release"], None, None)

          events.scheduler.add(build_task_finalize_handler, args)

  

@@ -19,7 +19,7 @@ 

  @celery_app.task

  @events.mbs_event_handler

  def build_task_finalize(

-         msg_id, build_id, task_id, build_new_state,

+         msg_id, task_id, build_new_state,

          build_name, build_version, build_release,

          module_build_id=None, state_reason=None

  ):
@@ -29,7 +29,6 @@ 

  

      :param str msg_id: the original id of the message being handled which is

          received from the message bus.

-     :param int build_id: the Koji build id.

      :param int task_id: the Koji build task id.

      :param int build_new_state: the state of the build. Refer to

          ``koji.BUILD_STATES`` for details. For this handler, values could be

@@ -14,13 +14,12 @@ 

  

  @celery_app.task

  @events.mbs_event_handler

- def tagged(msg_id, tag_name, build_name, build_nvr):

+ def tagged(msg_id, tag_name, build_nvr):

      """Called whenever koji tags a build to tag.

  

      :param str msg_id: the original id of the message being handled which is

          received from the message bus.

      :param str tag_name: the tag name applied.

-     :param str build_name: name of the tagged build.

      :param str build_nvr: nvr of the tagged build.

      """

      if conf.system not in ("koji", "test"):

@@ -78,7 +78,6 @@ 

                      return {

                          "msg_id": msg_id,

                          "event": events.KOJI_BUILD_CHANGE,

-                         "build_id": msg_inner_msg.get("build_id"),

                          "task_id": task_id,

                          "build_new_state": msg_inner_msg.get("new"),

                          "build_name": msg_inner_msg.get("name"),
@@ -106,7 +105,6 @@ 

                          "msg_id": msg_id,

                          "event": events.KOJI_TAG_CHANGE,

                          "tag_name": msg_inner_msg.get("tag"),

-                         "build_name": msg_inner_msg.get("name"),

                          "build_nvr": nvr,

                      }

  

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

                  log.info(

                      "Apply tag %s to module build %r",

                      module_build.koji_tag, module_build)

-                 tagged.delay(

-                     "internal:sync_koji_build_tags",

-                     module_build.koji_tag, c.package, c.nvr)

+                 tagged.delay("internal:sync_koji_build_tags", module_build.koji_tag, c.nvr)

  

              # If it is tagged in the build tag, but MBS does not think so,

              # schedule fake message.
@@ -433,9 +431,7 @@ 

                  log.info(

                      "Apply build tag %s to module build %r",

                      build_tag, module_build)

-                 tagged.delay(

-                     "internal:sync_koji_build_tags",

-                     build_tag, c.package, c.nvr)

+                 tagged.delay("internal:sync_koji_build_tags", build_tag, c.nvr)

  

  

  @celery_app.task

@@ -53,7 +53,7 @@ 

      # Add this event to scheduler so that the reused component will be tagged properly.

      if schedule_fake_events:

          args = (

-             "reuse_component: fake msg", None, component.task_id, previous_component_build.state,

+             "reuse_component: fake msg", component.task_id, previous_component_build.state,

              nvr_dict["name"], nvr_dict["version"], nvr_dict["release"], component.module_id,

              component.state_reason)

          events.scheduler.add(build_task_finalize_handler, args)

@@ -268,15 +268,9 @@ 

              FakeModuleBuilder.on_buildroot_add_artifacts_cb(self, artifacts, install)

          if self.backend == "test":

              for nvr in artifacts:

-                 # buildroot_add_artifacts received a list of NVRs, but the tag message expects the

-                 # component name. At this point, the NVR may not be set if we are trying to reuse

-                 # all components, so we can't search the database. We must parse the package name

-                 # from the nvr and then tag it in the build tag. Kobo doesn't work when parsing

-                 # the NVR of a component with a module dist-tag, so we must manually do it.

-                 package_name = nvr.split(".module")[0].rsplit("-", 2)[0]

                  # When INSTANT_COMPLETE is on, the components are already in the build tag

                  if self.INSTANT_COMPLETE is False:

-                     self._send_tag(package_name, nvr, dest_tag=False)

+                     self._send_tag(nvr, dest_tag=False)

          elif self.backend == "testlocal":

              self._send_repo_done()

  
@@ -292,10 +286,7 @@ 

              for nvr in artifacts:

                  # tag_artifacts received a list of NVRs, but the tag message expects the

                  # component name

-                 from sqlalchemy.orm import load_only

-                 artifact = self.db_session.query(models.ComponentBuild).filter_by(

-                     nvr=nvr).options(load_only("package")).first().package

-                 self._send_tag(artifact, nvr, dest_tag=dest_tag)

+                 self._send_tag(nvr, dest_tag=dest_tag)

  

      @property

      def koji_session(self):
@@ -317,17 +308,17 @@ 

      def _send_repo_done(self):

          events.scheduler.add(repos_done_handler, ("fake_msg", self.tag_name + "-build"))

  

-     def _send_tag(self, artifact, nvr, dest_tag=True):

+     def _send_tag(self, nvr, dest_tag=True):

          if dest_tag:

              tag = self.tag_name

          else:

              tag = self.tag_name + "-build"

-         events.scheduler.add(tagged_handler, ("a faked internal message", tag, artifact, nvr))

+         events.scheduler.add(tagged_handler, ("a faked internal message", tag, nvr))

  

      def _send_build_change(self, state, name, build_id):

          # build_id=1 and task_id=1 are OK here, because we are building just

          # one RPM at the time.

-         args = ("a faked internal message", build_id, build_id, state, name, "1", "1", None, None)

+         args = ("a faked internal message", build_id, state, name, "1", "1", None, None)

          events.scheduler.add(build_task_finalize_handler, args)

  

      def build(self, artifact_name, source):
@@ -368,7 +359,7 @@ 

              component_build.state_reason = "Found existing build"

              nvr_dict = kobo.rpmlib.parse_nvr(component_build.nvr)

              # Send a message stating the build is complete

-             args = ("recover_orphaned_artifact: fake message", randint(1, 9999999),

+             args = ("recover_orphaned_artifact: fake message",

                      component_build.task_id, koji.BUILD_STATES["COMPLETE"],

                      component_build.package, nvr_dict["version"], nvr_dict["release"],

                      component_build.module_build.id, None)
@@ -376,7 +367,7 @@ 

              # Send a message stating that the build was tagged in the build tag

              args = ("recover_orphaned_artifact: fake message",

                      component_build.module_build.koji_tag + "-build",

-                     component_build.package, component_build.nvr)

+                     component_build.nvr)

              events.scheduler.add(tagged_handler, args)

              return True

  
@@ -490,7 +481,7 @@ 

      @pytest.mark.parametrize("mmd_version", [1, 2])

      @patch("module_build_service.auth.get_user", return_value=user)

      @patch("module_build_service.scm.SCM")

-     def test_submit_build(

+     def test_submit_build_normal(

          self, mocked_scm, mocked_get_user, conf_system, dbg, hmsc, mmd_version

      ):

          """

@@ -155,16 +155,16 @@ 

          assert recovered

  

          event_info = events.scheduler.queue[0][3]

-         assert event_info == ('recover_orphaned_artifact: fake message', 91, 12345, 1,

+         assert event_info == ('recover_orphaned_artifact: fake message', 12345, 1,

                                'rubygem-rails', '1.0', '1.module+e0095747', 4, None)

  

          event_info = events.scheduler.queue[1][3]

          assert event_info == ('recover_orphaned_artifact: fake message', 'module-foo-build',

-                               'rubygem-rails', 'foo-1.0-1.module+e0095747')

+                               'foo-1.0-1.module+e0095747')

  

          event_info = events.scheduler.queue[2][3]

          assert event_info == ('recover_orphaned_artifact: fake message', 'module-foo',

-                               'rubygem-rails', 'foo-1.0-1.module+e0095747')

+                               'foo-1.0-1.module+e0095747')

  

          assert component_build.state == koji.BUILD_STATES["COMPLETE"]

          assert component_build.task_id == 12345
@@ -208,7 +208,7 @@ 

  

          assert recovered

          event_info = events.scheduler.queue[0][3]

-         assert event_info == ('recover_orphaned_artifact: fake message', 91, 12345, 1,

+         assert event_info == ('recover_orphaned_artifact: fake message', 12345, 1,

                                'rubygem-rails', '1.0', '1.module+2+b8661ee4', 4, None)

  

          assert component_build.state == koji.BUILD_STATES["COMPLETE"]
@@ -258,7 +258,7 @@ 

  

          assert recovered

          event_info = events.scheduler.queue[0][3]

-         assert event_info == ('recover_orphaned_artifact: fake message', 91, 12345, 1,

+         assert event_info == ('recover_orphaned_artifact: fake message', 12345, 1,

                                'module-build-macros', '1.0', "1.{0}".format(dist_tag), 4, None)

  

          assert component_build.state == koji.BUILD_STATES["COMPLETE"]

@@ -57,7 +57,7 @@ 

      def test_route_components_build_task_finalize_task(self, send_task_message):

          scheduler_init_data()

          components.build_task_finalize.delay(

-             "fakemsg", 123, 90276228, 1, "perl-Tangerine", "0.23", "1.module+f28+2+814cfa39")

+             "fakemsg", 90276228, 1, "perl-Tangerine", "0.23", "1.module+f28+2+814cfa39")

          queue = send_task_message.call_args[1].get("queue")

          qname = queue.__dict__.get("name")

          assert qname == "mbs-2"
@@ -65,7 +65,7 @@ 

      def test_route_components_build_task_finalize_task_without_a_module(self, send_task_message):

          scheduler_init_data()

          components.build_task_finalize.delay(

-             "fakemsg", 123, 123456, 1, "hostname", "0.1", "1.module+f28+2+814cfa39")

+             "fakemsg", 123456, 1, "hostname", "0.1", "1.module+f28+2+814cfa39")

          queue = send_task_message.call_args[1].get("queue")

          qname = queue.__dict__.get("name")

          assert qname == "mbs-default"
@@ -88,7 +88,7 @@ 

          scheduler_init_data()

          tags.tagged.delay(

              "fakemsg", "module-testmodule-master-20170109091357-7c29193d-build",

-             "perl-Tangerine", "perl-Tangerine-0.23-1.module+f28+2+814cfa39")

+             "perl-Tangerine-0.23-1.module+f28+2+814cfa39")

          queue = send_task_message.call_args[1].get("queue")

          qname = queue.__dict__.get("name")

          assert qname == "mbs-2"

file modified
-2
@@ -29,7 +29,6 @@ 

          parser = FedmsgMessageParser(messaging.known_fedmsg_services)

          event_info = parser.parse(buildsys_state_change_msg)

  

-         assert event_info["build_id"] == 614503

          assert event_info["build_new_state"] == 1

  

      def test_buildsys_tag(self):
@@ -55,7 +54,6 @@ 

          event_info = parser.parse(buildsys_tag_msg)

  

          assert event_info["tag_name"] == "module-debugging-tools-master-20170405115403-build"

-         assert event_info["build_name"] == "module-build-macros"

  

      def test_buildsys_repo_done(self):

          # https://fedora-fedmsg.readthedocs.io/en/latest/topics.html#id134

@@ -39,7 +39,6 @@ 

          assert event_info["event"] == events.KOJI_TAG_CHANGE

          assert event_info["msg_id"] == msg["msg_id"]

          assert event_info["tag_name"] == msg["msg"]["tag"]

-         assert event_info["build_name"] == msg["msg"]["name"]

  

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

      @patch.object(MBSConsumer, "process_message")

@@ -539,14 +539,14 @@ 

                      {"id": 1, "name": module_build_2.koji_tag + "-build"})

                  expected_tagged_calls.append(call(

                      "internal:sync_koji_build_tags",

-                     module_build_2.koji_tag + "-build", c.package, c.nvr

+                     module_build_2.koji_tag + "-build", c.nvr

                  ))

              if tagged_in_final:

                  listtags_return_value.append(

                      {"id": 2, "name": module_build_2.koji_tag})

                  expected_tagged_calls.append(call(

                      "internal:sync_koji_build_tags",

-                     module_build_2.koji_tag, c.package, c.nvr

+                     module_build_2.koji_tag, c.nvr

                  ))

          koji_session.listTags.return_value = listtags_return_value

  

@@ -27,7 +27,6 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="no matches for this...",

              tag_name="2016-some-nonexistent-build",

-             build_name="artifact",

              build_nvr="artifact-1.2-1")

  

      def test_no_matching_artifact(self):
@@ -37,7 +36,6 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="artifact",

              build_nvr="artifact-1.2-1",

          )

  
@@ -88,14 +86,12 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

          # Tag the first component to the final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

  
@@ -107,7 +103,6 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

  
@@ -119,7 +114,6 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

  
@@ -173,14 +167,12 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

          # Tag the perl-List-Compare component to final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

  
@@ -239,14 +231,12 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

          # Tag the perl-List-Compare component to final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

  
@@ -310,14 +300,12 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

          # Tag the first component to the final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

  
@@ -329,14 +317,12 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

          # Tag the second component to final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

  
@@ -348,14 +334,12 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="module-build-macros",

              build_nvr="module-build-macros-0.1-1.module+0+b0a1d1f7",

          )

          # Tag the component from first batch to the buildroot.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="module-build-macros",

              build_nvr="module-build-macros-0.1-1.module+0+b0a1d1f7",

          )

  
@@ -425,7 +409,6 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

          assert not koji_session.newRepo.called
@@ -433,14 +416,12 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

          # Tag the perl-List-Compare component to final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

  
@@ -518,28 +499,24 @@ 

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

          # Tag the first component to the final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-Tangerine",

              build_nvr="perl-Tangerine-0.23-1.module+0+d027b723",

          )

          # Tag the second component to the buildroot.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c-build",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

          # Tag the second component to the final tag.

          module_build_service.scheduler.handlers.tags.tagged(

              msg_id="id",

              tag_name="module-testmodule-master-20170219191323-c40c156c",

-             build_name="perl-List-Compare",

              build_nvr="perl-List-Compare-0.53-5.module+0+d027b723",

          )

  

file modified
+12 -12
@@ -1234,11 +1234,11 @@ 

          for event in events.scheduler.queue:

              event_info = event[3]

              if event_info[0].startswith("reuse_component"):

-                 assert event_info[3] == koji.BUILD_STATES["COMPLETE"]

+                 assert event_info[2] == koji.BUILD_STATES["COMPLETE"]

                  component_build = models.ComponentBuild.from_component_event(

                      db_session,

-                     task_id=event_info[2],

-                     module_id=event_info[7])

+                     task_id=event_info[1],

+                     module_id=event_info[6])

                  assert component_build.state == koji.BUILD_STATES["BUILDING"]

  

          # When we handle these KojiBuildChange messages, MBS should tag all
@@ -1284,13 +1284,13 @@ 

          # to BUILDING, so KojiBuildChange message handler handles the change

          # properly.

          event_info = events.scheduler.queue[0][3]

-         assert event_info == ('reuse_component: fake msg', None, 90276227, 1, 'perl-Tangerine',

+         assert event_info == ('reuse_component: fake msg', 90276227, 1, 'perl-Tangerine',

                                '0.23', '1.module+0+d027b723', 3,

                                'Reused component from previous module build')

          component_build = models.ComponentBuild.from_component_event(

              db_session,

-             task_id=event_info[2],

-             module_id=event_info[7],

+             task_id=event_info[1],

+             module_id=event_info[6],

          )

          assert component_build.state == koji.BUILD_STATES["BUILDING"]

          assert component_build.package == "perl-Tangerine"
@@ -1379,13 +1379,13 @@ 

          # in the DB should be set to BUILDING, so the build state change handler

          # handles the change properly.

          event_info = events.scheduler.queue[0][3]

-         assert event_info == ('reuse_component: fake msg', None, 90276227, 1,

+         assert event_info == ('reuse_component: fake msg', 90276227, 1,

                                'perl-Tangerine', '0.23', '1.module+0+d027b723', 3,

                                'Reused component from previous module build')

          component_build = models.ComponentBuild.from_component_event(

              db_session,

-             task_id=event_info[2],

-             module_id=event_info[7],

+             task_id=event_info[1],

+             module_id=event_info[6],

          )

          assert component_build.state == koji.BUILD_STATES["BUILDING"]

          assert component_build.package == "perl-Tangerine"
@@ -1412,13 +1412,13 @@ 

          # Verify that tangerine was reused even though perl-Tangerine was rebuilt in the previous

          # batch

          event_info = events.scheduler.queue[0][3]

-         assert event_info == ('reuse_component: fake msg', None, 90276315, 1, 'tangerine', '0.22',

+         assert event_info == ('reuse_component: fake msg', 90276315, 1, 'tangerine', '0.22',

                                '3.module+0+d027b723', 3,

                                'Reused component from previous module build')

          component_build = models.ComponentBuild.from_component_event(

              db_session,

-             task_id=event_info[2],

-             module_id=event_info[7],

+             task_id=event_info[1],

+             module_id=event_info[6],

          )

          assert component_build.state == koji.BUILD_STATES["BUILDING"]

          assert component_build.package == "tangerine"