From f098e6e3b771ed3497f2d731e50185fcbea63133 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Tue, 7 Nov 2017 00:04:14 +0100 Subject: [PATCH] Make the module build log name configurable --- conf/config.py | 1 + module_build_service/__init__.py | 2 +- module_build_service/config.py | 4 +++ module_build_service/logger.py | 30 ++++++++++------- .../scheduler/handlers/modules.py | 6 ++-- tests/test_content_generator.py | 4 +-- tests/test_logger.py | 32 ++++++++++++++----- 7 files changed, 54 insertions(+), 25 deletions(-) diff --git a/conf/config.py b/conf/config.py index a97fcda8..db528b99 100644 --- a/conf/config.py +++ b/conf/config.py @@ -142,6 +142,7 @@ class DevConfiguration(BaseConfiguration): class TestConfiguration(BaseConfiguration): BUILD_LOGS_DIR = '/tmp' + BUILD_LOGS_NAME_FORMAT = 'build-{id}.log' LOG_BACKEND = 'console' LOG_LEVEL = 'debug' SQLALCHEMY_DATABASE_URI = 'sqlite:///{0}'.format( diff --git a/module_build_service/__init__.py b/module_build_service/__init__.py index ceecb59e..b8e638ae 100644 --- a/module_build_service/__init__.py +++ b/module_build_service/__init__.py @@ -128,7 +128,7 @@ def notfound_error(e): init_logging(conf) log = getLogger(__name__) -build_logs = ModuleBuildLogs(conf.build_logs_dir) +build_logs = ModuleBuildLogs(conf.build_logs_dir, conf.build_logs_name_format) def get_url_for(*args, **kwargs): diff --git a/module_build_service/config.py b/module_build_service/config.py index ce89c725..ae6cddba 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -253,6 +253,10 @@ class Config(object): 'type': Path, 'default': "", 'desc': 'Directory to store module build logs to.'}, + 'build_logs_name_format': { + 'type': str, + 'default': "build-{id}.log", + 'desc': 'Format of a module build log\'s name. Use `Build` attributes as formatting kwargs'}, 'krb_keytab': { 'type': None, 'default': None, diff --git a/module_build_service/logger.py b/module_build_service/logger.py index 228376eb..838dc3a0 100644 --- a/module_build_service/logger.py +++ b/module_build_service/logger.py @@ -87,55 +87,63 @@ class ModuleBuildLogs(object): """ Manages ModuleBuildFileHandler logging handlers. """ - def __init__(self, build_logs_dir): + def __init__(self, build_logs_dir, build_logs_name_format): """ Creates new ModuleBuildLogs instance. Module build logs are stored to `build_logs_dir` directory. """ self.handlers = {} self.build_logs_dir = build_logs_dir + self.build_logs_name_format = build_logs_name_format - def path(self, build_id): + def path(self, build): """ Returns the full path to build log of module with id `build_id`. """ - path = os.path.join(self.build_logs_dir, "build-%d.log" % build_id) + path = os.path.join(self.build_logs_dir, self.name(build)) return path - def start(self, build_id): + def name(self, build): + """ + Returns the filename for a module build + """ + name = self.build_logs_name_format.format(**build.json()) + return name + + def start(self, build): """ Starts logging build log for module with `build_id` id. """ if not self.build_logs_dir: return - if build_id in self.handlers: + if build.id in self.handlers: return # Create and add ModuleBuildFileHandler. - handler = ModuleBuildFileHandler(build_id, self.path(build_id)) + handler = ModuleBuildFileHandler(build.id, self.path(build)) handler.setFormatter(logging.Formatter(log_format, None)) log = logging.getLogger() log.addHandler(handler) - self.handlers[build_id] = handler + self.handlers[build.id] = handler - def stop(self, build_id): + def stop(self, build): """ Stops logging build log for module with `build_id` id. It does *not* remove the build log from fs. """ - if build_id not in self.handlers: + if build.id not in self.handlers: return - handler = self.handlers[build_id] + handler = self.handlers[build.id] handler.flush() handler.close() # Remove the log handler. log = logging.getLogger() log.removeHandler(handler) - del self.handlers[build_id] + del self.handlers[build.id] def str_to_log_level(level): diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index c8473230..30a8b8ab 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -95,7 +95,7 @@ def failed(config, session, msg): build.transition(config, state="failed") session.commit() - build_logs.stop(build.id) + build_logs.stop(build) module_build_service.builder.GenericBuilder.clear_cache(build) @@ -123,7 +123,7 @@ def done(config, session, msg): build.transition(config, state="ready") session.commit() - build_logs.stop(build.id) + build_logs.stop(build) module_build_service.builder.GenericBuilder.clear_cache(build) @@ -178,7 +178,7 @@ def wait(config, session, msg): .format(build)) build = _get_build_containing_xmd_for_mbs() - build_logs.start(build.id) + build_logs.start(build) log.info("Found build=%r from message" % build) log.info("%r", build.modulemd) diff --git a/tests/test_content_generator.py b/tests/test_content_generator.py index 5a13f87b..85d08c09 100644 --- a/tests/test_content_generator.py +++ b/tests/test_content_generator.py @@ -113,8 +113,8 @@ class TestBuild(unittest.TestCase): expected_output = json.load(expected_output_file) # create the build.log - build_logs.start(self.cg.module.id) - build_logs.stop(self.cg.module.id) + build_logs.start(self.cg.module) + build_logs.stop(self.cg.module) file_dir = self.cg._prepare_file_directory() ret = self.cg._get_content_generator_metadata(file_dir) diff --git a/tests/test_logger.py b/tests/test_logger.py index 86be04d6..4a4d92af 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -26,17 +26,20 @@ import shutil import tempfile import unittest -from module_build_service import log +from module_build_service import log, models from module_build_service.logger import ModuleBuildLogs from module_build_service.scheduler.consumer import MBSConsumer +from tests import init_data class TestLogger(unittest.TestCase): def setUp(self): + init_data() self.base = tempfile.mkdtemp(prefix='mbs-', suffix='-%s' % self.id()) + self.name_format = "build-{id}.log" print("Storing build logs in %r" % self.base) - self.build_log = ModuleBuildLogs(self.base) + self.build_log = ModuleBuildLogs(self.base, self.name_format) def tearDown(self): MBSConsumer.current_module_build_id = None @@ -46,10 +49,12 @@ class TestLogger(unittest.TestCase): """ Tests that ModuleBuildLogs is logging properly to build log file. """ + build = models.ModuleBuild.query.filter_by(id=1).one() + # Initialize logging, get the build log path and remove it to # ensure we are not using some garbage from previous failed test. - self.build_log.start(1) - path = self.build_log.path(1) + self.build_log.start(build) + path = self.build_log.path(build) self.assertEqual(path[len(self.base):], "/build-1.log") if os.path.exists(path): os.unlink(path) @@ -60,13 +65,13 @@ class TestLogger(unittest.TestCase): log.info("ignore this test msg") log.warn("ignore this test msg") log.error("ignore this test msg") - self.build_log.stop(1) + self.build_log.stop(build) self.assertTrue(not os.path.exists(path)) # Try logging with current_module_build_id set to 1 and then to 2. # Only messages with current_module_build_id set to 1 should appear in # the log. - self.build_log.start(1) + self.build_log.start(build) MBSConsumer.current_module_build_id = 1 log.debug("ignore this test msg1") log.info("ignore this test msg1") @@ -79,7 +84,7 @@ class TestLogger(unittest.TestCase): log.warn("ignore this test msg2") log.error("ignore this test msg2") - self.build_log.stop(1) + self.build_log.stop(build) self.assertTrue(os.path.exists(path)) with open(path, "r") as f: data = f.read() @@ -93,7 +98,18 @@ class TestLogger(unittest.TestCase): log.info("ignore this test msg3") log.warn("ignore this test msg3") log.error("ignore this test msg3") - self.build_log.stop(1) + self.build_log.stop(build) with open(path, "r") as f: data = f.read() self.assertTrue(data.find("ignore this test msg3") == -1) + + def test_module_build_logs_name_format(self): + build = models.ModuleBuild.query.filter_by(id=1).one() + + log1 = ModuleBuildLogs("/some/path", "build-{id}.log") + self.assertEquals(log1.name(build), "build-1.log") + self.assertEquals(log1.path(build), "/some/path/build-1.log") + + log2 = ModuleBuildLogs("/some/path", "build-{name}-{stream}-{version}.log") + self.assertEquals(log2.name(build), "build-nginx-1-2.log") + self.assertEquals(log2.path(build), "/some/path/build-nginx-1-2.log")