reframe-hpc/reframe

Can't set logging.handlers.file.timestamp to true

brandonbiggs opened this issue ยท 3 comments

I'm running reframe 4.2.1 so this may have been fixed, but I went through the release notes and didn't see anything about it..

In the logging configuration documentation for file handlers it indicates I can set timestamp = True but when I tried that, I received the following error.

Example config:

    'logging': [
        {
            'level': 'debug',
            'handlers': [
                {
                    'type': 'file',
                    'name': 'test_results/reframe-debug.log',
                    'level': 'debug',
                    'format': '[%(asctime)s] %(levelname)s: %(check_info)s: %(message)s', 
                    'append': True,
                    'timestamp': True
                }
            ]
       }
    ]

Error:

Traceback (most recent call last):
  File "/path/reframe/bin/reframe", line 22, in <module>
    cli.main()
  File "/path/reframe/reframe/core/logging.py", line 968, in _fn
    return fn(*args, **kwargs)
  File "/path/reframe/reframe/frontend/cli.py", line 776, in main
    logging.configure_logging(site_config)
  File "/path/reframe/reframe/core/logging.py", line 921, in configure_logging
    _logger = _create_logger(site_config, 'handlers$', 'handlers')
  File "/path/reframe/reframe/core/logging.py", line 348, in _create_logger
    for handler in _extract_handlers(site_config, hgrp):
  File "/path/reframe/reframe/core/logging.py", line 629, in _extract_handlers
    hdlr = _create_file_handler(site_config, f'{handler_prefix}/{i}')
  File "/path/reframe/reframe/core/logging.py", line 369, in _create_file_handler
    filename = f'{basename}_{time.strftime(timestamp)}{ext}'
TypeError: strftime() argument 1 must be str, not bool

Easy enough to fix as I just set it to a timestamp string like %Y%m%d, but documentation might need to change or change code to allow for bool.

I think that's present even in the latest. I've hit that too. Thanks for reporting. Indeed, the workaround is to set a date format. Maybe the docs are wrong here. ๐Ÿ‘€

Updating the docs would definitely be the easiest way to fix this. Do you want people to be able to set it to True and have it default to some default timestamp though?

This is indeed a bug as even the config schema accepts boolean values for timestamp.