#997 backend: avoid excception-hell in check_build_success()
Merged 2 months ago by msuchy. Opened 2 months ago by praiskup.
copr/ praiskup/copr exception-for-success-file  into  master

@@ -213,9 +213,10 @@ 

  

                  except MockRemoteError as e:

                      # record and break

-                     self.log.exception(

-                         "Error during the build, host=%s, build_id=%s, chroot=%s",

-                         self.vm.vm_ip, job.build_id, job.chroot)

+                     self.log.error(

+                         "Error during the build, host=%s, build_id=%s, "

+                         "chroot=%s, error='%s'",

+                         self.vm.vm_ip, job.build_id, job.chroot, str(e))

                      failed = True

                      mr.download_results()

  

file modified
+5 -6

@@ -396,14 +396,13 @@ 

          # copr specific semantics

          record.who = self.who

  

+         # For the message arguments, it is better to expand them right now

+         # instead of relying on method in json.dumps(..., default=default)

+         # and even worse rely on it's reverse action in RedisLogHandler.

+         record.msg = record.msg % record.args

          if record.exc_info:

              _, error, tb = record.exc_info

-             record.msg = format_tb(error, tb)

-         else:

-             # For the message arguments, it is better to expand them right now

-             # instead of relying on method in json.dumps(..., default=default)

-             # and even worse rely on it's reverse action in RedisLogHandler.

-             record.msg = record.msg % record.args

+             record.msg += "\n" + format_tb(error, tb)

  

          # cleanup the hard to json.dumps() stuff

          record.exc_info = None

@@ -306,9 +306,10 @@ 

              raise MockRemoteError("Builder error during job {}.".format(self.job))

  

      def check_build_success(self):

-         try:

-             self.builder.check_build_success()

-         except BuilderError as error:

+         """

+         Raise MockRemoteError if builder claims that the build failed.

+         """

+         if not self.builder.check_build_success():

              raise MockRemoteError("Build {} failed".format(self.job))

  

      def download_results(self):

@@ -56,8 +56,11 @@ 

          return out, err

  

      def check_build_success(self):

+         """

+         Check if the build succeeded.  If yes, return True.

+         """

          successfile = os.path.join(self.resultdir, "success")

-         self._run_ssh_cmd("/usr/bin/test -f {0}".format(successfile))

+         return self.conn.run("/usr/bin/test -f {0}".format(successfile)) == 0

  

      def run_async_build(self):

          cmd = self._copr_builder_cmd()

Previously, we expected that Builder.check_build_success() raised
BuilderError in case of error, but this never happened - we instead
raised RemoteCmdError which was never catched.

Instead of cathing RemoteCmdError, execute conn.run() directly; it is
both faster (we ignore stdout/stderr) and both we can avoid the
exception handling mesh in every call.

The only exception which could be raised there is SSHConnectionError,
which is carefully catched in worker.py.

Relates: #987

1 new commit added

  • backend: don't leave tracebacks in exception log entries alone
2 months ago

1 new commit added

  • backend: lower the MockRemoteError exception to error
2 months ago

rebased onto 79ba31a

2 months ago

Pull-Request has been merged by msuchy

2 months ago