#1675 Don't traceback on failed builds
Merged 2 years ago by breilly. Opened 3 years ago by otaylor.
otaylor/fm-orchestrator failed-build-traceback  into  master

@@ -5,6 +5,7 @@ 

  import getpass

  import logging

  import os

+ import sys

  import textwrap

  

  import flask_migrate
@@ -194,15 +195,11 @@ 

  

          module_build_ids = [build.id for build in module_builds]

  

-     module_build_service.scheduler.local.main(module_build_ids)

- 

-     has_failed_module = db_session.query(models.ModuleBuild).filter(

-         models.ModuleBuild.id.in_(module_build_ids),

-         models.ModuleBuild.state == models.BUILD_STATES["failed"],

-     ).count() > 0

- 

-     if has_failed_module:

-         raise RuntimeError("Module build failed")

+     try:

+         module_build_service.scheduler.local.main(module_build_ids)

+     except module_build_service.scheduler.local.BuildFailed as e:

+         logging.error("%s", e)

+         sys.exit(1)

  

  

  @manager.option(

@@ -21,6 +21,10 @@ 

  __all__ = ["main"]

  

  

+ class BuildFailed(Exception):

+     pass

+ 

+ 

  def raise_for_failed_build(module_build_ids):

      """

      Raises an exception if any module build from `module_build_ids` list is in failed state.
@@ -36,7 +40,7 @@ 

              modules_failed_handler("fake_msg_id", build.id, "failed")

              has_failed_build = True

      if has_failed_build:

-         raise ValueError("Local module build failed.")

+         raise BuildFailed("Local module build failed.")

  

  

  def main(module_build_ids):

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

  # SPDX-License-Identifier: MIT

  from __future__ import absolute_import

  

+ import logging

  from mock import patch

  import pytest

  
@@ -205,14 +206,14 @@ 

          args, _ = logging.error.call_args_list[1]

          assert "Use '-s module_name:module_stream' to choose the stream" == args[0]

  

-     def test_module_build_failed(self):

+     def test_module_build_failed(self, caplog):

          cli_cmd = [

              "mbs-manager", "build_module_locally",

              "--set-stream", "platform:f28",

              "--file", staged_data_filename("testmodule-local-build.yaml")

          ]

  

-         def main_side_effect(module_build_ids):

+         def init_side_effect(*args):

              build = db_session.query(models.ModuleBuild).filter(

                  models.ModuleBuild.name == "testmodule-local-build"

              ).first()
@@ -222,7 +223,10 @@ 

          # We don't run consumer actually, but it could be patched to mark some

          # module build failed for test purpose.

  

-         with patch("module_build_service.scheduler.local.main",

-                    side_effect=main_side_effect):

-             with pytest.raises(RuntimeError, match="Module build failed"):

-                 self._run_manager_wrapper(cli_cmd)

+         with patch("module_build_service.scheduler.local.modules_init_handler",

+                    side_effect=init_side_effect):

+             self._run_manager_wrapper(cli_cmd)

+ 

+         r = [r for r in caplog.records if r.levelno == logging.ERROR

+              and "Local module build failed" in r.getMessage()]

+         assert len(r) > 0

Just failing a build isn't a reason to die with a traceback - exit with
a status instead.

Fixes: #1364

rebased onto e5c51d9

2 years ago

Commit 3231d35 fixes this pull-request

Pull-Request has been merged by breilly

2 years ago

Pull-Request has been merged by breilly

2 years ago