Merge pull request #445 from vkarak/bugfix/config-logdir

[bugfix] Fix reading of 'logdir' setting from system config

Author: vkarak

docs/configure.rst

@@ -103,7 +103,8 @@ The valid attributes of a system are the following:
 * ``prefix``: Default regression prefix for this system (default ``.``).
 * ``stagedir``: Default stage directory for this system (default :class:`None`).
 * ``outputdir``: Default output directory for this system (default :class:`None`).
-* ``logdir``: Default performance logging directory for this system (default :class:`None`).
+* ``perflogdir``: Default directory prefix for storing performance logs for this system (default :class:`None`).
+* ``logdir``: `Deprecated since version 2.14 please use` ``perflogdir`` `instead.`
 * ``resourcesdir``: Default directory for storing large resources (e.g., input data files, etc.) needed by regression tests for this system (default ``.``).
 * ``partitions``: A set of key/value pairs defining the partitions of this system and their properties (default ``{}``).
   Partition configuration is discussed in the `next section <#partition-configuration>`__.
@@ -112,7 +113,7 @@ The valid attributes of a system are the following:
   .. versionadded:: 2.8
     The ``modules_system`` key was introduced for specifying custom modules systems for different systems.
 
-For a more detailed description of the ``prefix``, ``stagedir``, ``outputdir`` and ``logdir`` directories, please refer to the `"Running ReFrame" <running.html#configuring-reframe-directories>`__ section.
+For a more detailed description of the ``prefix``, ``stagedir``, ``outputdir`` and ``perflogdir`` directories, please refer to the `"Configuring ReFrame Directories" <running.html#configuring-reframe-directories>`__ and `"Performance Logging" <running.html#performance-logging>`__ sections.
 
 Partition Configuration
 -----------------------

docs/running.rst

@@ -625,7 +625,7 @@ The attributes of this handler are the following:
 * ``prefix``: This is the directory prefix (usually dynamic) where the performance logs of a test will be stored.
   This attribute accepts any of the check-specific formatting placeholders described `above <#common-log-handler-attributes>`__.
   This allows you to create dynamic paths based on the current system, partition and/or programming environment a test executes.
-  This dynamic prefix is appended to the "global" performance log directory prefix, configurable through the ``--perflogdir`` option.
+  This dynamic prefix is appended to the "global" performance log directory prefix, configurable through the ``--perflogdir`` option or the ``perflogdir`` attribute of the `system configuration <configuring.html#system-configuration>`__.
   The default configuration of ReFrame for performance logging (shown in the previous listing) generates the following files:
 
   .. code-block:: none
@@ -719,7 +719,7 @@ This handler introduces three new attributes:
 This log handler uses internally `pygelf <https://pypi.org/project/pygelf/>`__, so this Python module must be available, otherwise this log handler will be ignored.
 `GELF <http://docs.graylog.org/en/latest/pages/gelf.html>`__ is a format specification for log messages that are sent over the network.
 The ReFrame's ``graylog`` handler sends log messages in JSON format using an HTTP POST request to the specified host and port.
-More details on this log format may be found `here <http://docs.graylog.org/en/latest/pages/gelf.html#gelf-payload-specification>`__
+More details on this log format may be found `here <http://docs.graylog.org/en/latest/pages/gelf.html#gelf-payload-specification>`__.
 
 
 Asynchronous Execution of Regression Checks

reframe/core/config.py

@@ -4,7 +4,10 @@
 import reframe.core.debug as debug
 import reframe.core.fields as fields
 import reframe.utility as util
-from reframe.core.exceptions import ConfigError, ReframeError, ReframeFatalError
+from reframe.core.exceptions import (ConfigError,
+                                     ReframeError,
+                                     ReframeFatalError,
+                                     user_deprecation_warning)
 
 
 _settings = None
@@ -135,11 +138,12 @@ def create_env(system, partition, name):
             # The System's constructor provides also reasonable defaults, but
             # since we are going to set them anyway from the values provided by
             # the configuration, we should set default values here. The stage,
-            # output and log directories default to None, since they are
-            # going to be set dynamically by the ResourcesManager
+            # output and log directories default to None, since they are going
+            # to be set dynamically by the runtime.
             sys_prefix = config.get('prefix', '.')
             sys_stagedir = config.get('stagedir', None)
             sys_outputdir = config.get('outputdir', None)
+            sys_perflogdir = config.get('perflogdir', None)
             sys_logdir = config.get('logdir', None)
             sys_resourcesdir = config.get('resourcesdir', '.')
             sys_modules_system = config.get('modules_system', None)
@@ -155,7 +159,14 @@ def create_env(system, partition, name):
                 sys_outputdir = os.path.expandvars(sys_outputdir)
 
             if sys_logdir:
-                sys_logdir = os.path.expandvars(sys_logdir)
+                user_deprecation_warning(
+                    "`logdir' attribute in system config is deprecated; "
+                    "please use `perflogdir' instead"
+                )
+                sys_perflogdir = os.path.expandvars(sys_logdir)
+
+            if sys_perflogdir:
+                sys_perflogdir = os.path.expandvars(sys_perflogdir)
 
             if sys_resourcesdir:
                 sys_resourcesdir = os.path.expandvars(sys_resourcesdir)
@@ -166,7 +177,7 @@ def create_env(system, partition, name):
                             prefix=sys_prefix,
                             stagedir=sys_stagedir,
                             outputdir=sys_outputdir,
-                            logdir=sys_logdir,
+                            perflogdir=sys_perflogdir,
                             resourcesdir=sys_resourcesdir,
                             modules_system=sys_modules_system)
             for part_name, partconfig in config.get('partitions', {}).items():

reframe/core/runtime.py

@@ -94,14 +94,16 @@ class HostResources:
     prefix = fields.AbsolutePathField('prefix')
     outputdir = fields.AbsolutePathField('outputdir', allow_none=True)
     stagedir  = fields.AbsolutePathField('stagedir', allow_none=True)
+    perflogdir = fields.AbsolutePathField('perflogdir', allow_none=True)
 
     def __init__(self, prefix=None, stagedir=None,
-                 outputdir=None, timefmt=None):
+                 outputdir=None, perflogdir=None, timefmt=None):
         self.prefix = prefix or '.'
         self.stagedir  = stagedir
         self.outputdir = outputdir
-        self._timestamp = datetime.now()
+        self.perflogdir = perflogdir
         self.timefmt = timefmt
+        self._timestamp = datetime.now()
 
     def _makedir(self, *dirs, wipeout=False):
         ret = os.path.join(*dirs)
@@ -131,6 +133,13 @@ def stage_prefix(self):
         else:
             return os.path.join(self.stagedir, self.timestamp)
 
+    @property
+    def perflog_prefix(self):
+        if self.perflogdir is None:
+            return os.path.join(self.prefix, 'perflogs')
+        else:
+            return self.perflogdir
+
     def make_stagedir(self, *dirs, wipeout=True):
         return self._makedir(self.stage_prefix, *dirs, wipeout=wipeout)
 
@@ -165,7 +174,7 @@ def __init__(self, dict_config, sysdescr=None):
 
         self._resources = HostResources(
             self._system.prefix, self._system.stagedir,
-            self._system.outputdir, self._system.logdir)
+            self._system.outputdir, self._system.perflogdir)
         self._modules_system = ModulesSystem.create(
             self._system.modules_system)
 

reframe/core/systems.py

@@ -154,11 +154,11 @@ class System:
     _prefix = fields.StringField('_prefix')
     _stagedir  = fields.StringField('_stagedir', allow_none=True)
     _outputdir = fields.StringField('_outputdir', allow_none=True)
-    _logdir = fields.StringField('_logdir', allow_none=True)
+    _perflogdir = fields.StringField('_perflogdir', allow_none=True)
     _resourcesdir = fields.StringField('_resourcesdir')
 
     def __init__(self, name, descr=None, hostnames=[], partitions=[],
-                 prefix='.', stagedir=None, outputdir=None, logdir=None,
+                 prefix='.', stagedir=None, outputdir=None, perflogdir=None,
                  resourcesdir='.', modules_system=None):
         self._name  = name
         self._descr = descr or name
@@ -168,7 +168,7 @@ def __init__(self, name, descr=None, hostnames=[], partitions=[],
         self._prefix = prefix
         self._stagedir = stagedir
         self._outputdir = outputdir
-        self._logdir = logdir
+        self._perflogdir = perflogdir
         self._resourcesdir = resourcesdir
 
         # Set parent system for the given partitions
@@ -211,9 +211,9 @@ def outputdir(self):
         return self._outputdir
 
     @property
-    def logdir(self):
+    def perflogdir(self):
         """The ReFrame log directory prefix associated with this system."""
-        return self._logdir
+        return self._perflogdir
 
     @property
     def resourcesdir(self):

reframe/frontend/cli.py

@@ -297,11 +297,10 @@ def main():
     # NOTE: we need resources to be configured in order to set the global
     # perf. logging prefix correctly
     if options.perflogdir:
-        logging.LOG_CONFIG_OPTS['handlers.filelog.prefix'] = (
-            os.path.expandvars(options.perflogdir))
-    else:
-        logging.LOG_CONFIG_OPTS['handlers.filelog.prefix'] = (
-            os.path.join(rt.resources.prefix, 'perflogs'))
+        rt.resources.perflogdir = os.path.expandvars(options.perflogdir)
+
+    logging.LOG_CONFIG_OPTS['handlers.filelog.prefix'] = (rt.resources.
+                                                          perflog_prefix)
 
     if hasattr(settings, 'perf_logging_config'):
         try:

unittests/resources/settings.py

@@ -31,8 +31,9 @@ class ReframeSettings:
                 # order to test different aspects of the framework.
                 'descr': 'Fake system for unit tests',
                 'hostnames': ['testsys'],
-                'prefix': '.rfm_testing/install',
+                'prefix': '.rfm_testing',
                 'resourcesdir': '.rfm_testing/resources',
+                'perflogdir': '.rfm_testing/perflogs',
                 'partitions': {
                     'login': {
                         'scheduler': 'local',

unittests/test_config.py

@@ -22,6 +22,9 @@ def test_load_success(self):
 
         system = self.site_config.systems['testsys']
         self.assertEqual(2, len(system.partitions))
+        self.assertEqual('.rfm_testing', system.prefix)
+        self.assertEqual('.rfm_testing/resources', system.resourcesdir)
+        self.assertEqual('.rfm_testing/perflogs', system.perflogdir)
 
         part_login = self.get_partition(system, 'login')
         part_gpu = self.get_partition(system, 'gpu')

unittests/test_runtime.py

@@ -10,11 +10,12 @@ def test_hostsystem_api(self):
         system = rt.runtime().system
         self.assertEqual('testsys', system.name)
         self.assertEqual('Fake system for unit tests', system.descr)
-        self.assertEqual('.rfm_testing/resources', system.resourcesdir)
         self.assertEqual(2, len(system.partitions))
         self.assertIsNotNone(system.partition('login'))
         self.assertIsNotNone(system.partition('gpu'))
         self.assertIsNone(system.partition('foobar'))
 
         # Test delegation to the underlying System
-        self.assertEqual('.rfm_testing/install', system.prefix)
+        self.assertEqual('.rfm_testing', system.prefix)
+        self.assertEqual('.rfm_testing/resources', system.resourcesdir)
+        self.assertEqual('.rfm_testing/perflogs', system.perflogdir)