#788 [backend] compress backend-live.log and make redirect to compressed file, Closes #86
Merged 4 months ago by praiskup. Opened 5 months ago by hojang.
copr/ hojang/copr log_compression  into  master

@@ -207,6 +207,7 @@ 

                          mr.check()

                          mr.build_pkg()

  

+                     mr.compress_live_log(job)

                      mr.check_build_success() # raises if build didn't succeed

                      mr.download_results()

  

@@ -17,6 +17,7 @@ 

  import fcntl

  import logging

  import os

+ import subprocess

  

  from munch import Munch

  import time

@@ -273,6 +274,21 @@ 

              self.log.error(str(error))

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

  

+     def compress_live_log(self, job):

+         log_basename = "builder-live.log"

+         src = os.path.join(job.results_dir, log_basename)

+         

+         self.log.info("Compressing {} by gzip".format(src))

+         if subprocess.call(["gzip", src]) not in [0, 2]:

+             self.log.error("Unable to compress file {}".format(src))

+             return

+ 

+         try:

+             with open(src, "w") as f_src:

+                 f_src.write("{} was moved to {}.gz\n".format(log_basename, log_basename))

+         except Exception as e:

+             self.log.exception(e)

+ 

      def reattach_to_pkg_build(self):

          """

          :raises VmError, BuilderError

Because we want to use less space, we can compress backend-live.log after a build.

please wrap this into separate method, probably in MockRemote. I think that we should compress even failed logs, so perhaps move this before check_build_success()?

Also make sure that it never throws exceptions; we can afford failures here.

The files look like:

..snip../00921247-copr-backend/builder-live.log
..snip../srpm-builds/00921248/builder-live.log

This will redirect always. We want to redirect only conditionally when the .log file does not exist.

rebased onto 16112a1c3a96fb92a02c577e33b5b5ce82f68a24

5 months ago

Typo, but I believe it is meant correctly: we want to redirect only if the .log.gz file exists.

Metadata Update from @praiskup:
- Pull-request tagged with: needs-work

5 months ago

rebased onto 2a3a5bc368b60322a99eba9aaba81d233fe936c2

5 months ago

I'd probably prefer simply calling subprocess.call(['gzip', 'file']). Or alternatively we should have catch-all exception handling here, because we don't want to fail the process for this low-prio thing.

Please avoid taking care of the configuration in this PR; it is pitty to keep it open when it is almost ready :-)

rebased onto 92e1e8a5f3d62e991a9cdd76bd2be684921c1aa7

4 months ago

Please call the gzip binary directly, by subprocess.call(['gzip', src]).

Nice, please add newline here. ("\n" missing)

rebased onto df66d6a415aa006252ac1459c0b4eaa1be44e722

4 months ago

There's no exception here, so please use log.error.

two nits, but otherwise looks good to me ... I'll try to deploy this to staging

The dst variable is not defined, what about to just write:
"build finished, build-live.log was compressed and moved to build-live.log.gz"

the != 0 isn't needed here.
Well, subprocess.call returns int, not boolean.

rebased onto 6f007bc465e3a6d2ee5c7a58c501046996f4280f

4 months ago

rebased onto bca5a6f1b9899b2a008ffd52dd9814299f3a4165

4 months ago

looks good, gzip returns 2 when there's some warning

This produces:
/var/lib/copr/public_html/results/praiskup/ping/srpm-builds/00842157/builder-live.log was moved to /var/lib/copr/public_html/results/praiskup/ping/srpm-builds/00842157/builder-live.log.gz

I'd prefer just builder-live.log was moved to builder-live.log.gz variant.

rebased onto c0d3cbd669ea1bf85c997a851631abbd0dd1eb34

4 months ago

awesome! temporarily on staging: https://copr-fe-dev.cloud.fedoraproject.org/coprs/praiskup/ping/build/842178/

Please change the commit message so it doesn't mention the "redirect" part, it's not truth anymore; otherwise good.

rebased onto 1dab92b47091f967fe2add2d740c6e8ecbf13764

4 months ago

Metadata Update from @praiskup:
- Pull-request untagged with: needs-work

4 months ago

rebased onto 75b5a8f34c2263bcd6d6a289372c9c44a9751276

4 months ago

rebased onto 78553d3

4 months ago

Pull-Request has been merged by praiskup

4 months ago