From de46b0bf6bc995678dd688ed1dd69f10ddb0595e Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 17 Aug 2025 16:18:53 -0400 Subject: [PATCH 01/26] close branch From bb19835178949bb40bc47e30917b812c969f93dd Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 17 Aug 2025 16:35:46 -0400 Subject: [PATCH 02/26] close duplicate head caused by identical merge in two working copies From 13134bd4631ff1f97372177cfb906504d98dd1c4 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 17 Aug 2025 16:44:22 -0400 Subject: [PATCH 03/26] document the repo cleanups and duplicate merge to get things cleaned up --- CHANGES.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 122e8352..b181e066 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -23,6 +23,9 @@ Fixed: roundup.cgi.exceptions. Also it now inherits from HTTPException rather than Exception since it is an HTTP exception. (John Rouillard) +- cleaned up repo. Close obsolete branches and close a split head due + to an identical merge intwo different workicng copies. (John + Rouillard) Features: From 28b5642f812659cf5de1051a741c05f99ceebe7b Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 19 Aug 2025 22:32:46 -0400 Subject: [PATCH 04/26] feat: add support for using dictConfig to configure logging. Basic logging config (one level and one output file non-rotating) was always possible from config.ini. However the LOGGING_CONFIG setting could be used to load an ini fileConfig style file to set various channels (e.g. roundup.hyperdb) (also called qualname or tags) with their own logging level, destination (rotating file, socket, /dev/null) and log format. This is now a deprecated method in newer logging modules. The dictConfig format is preferred and allows disabiling other loggers as well as invoking new loggers in local code. This commit adds support for it reading the dict from a .json file. It also implements a comment convention so you can document the dictConfig. configuration.py: new code test_config.py: test added for the new code. admin_guide.txt, upgrading.txt CHANGES.txt: docs added upgrading references the section in admin_guid. --- CHANGES.txt | 2 + doc/admin_guide.txt | 275 +++++++++++++++++++++++++++++++++++++-- doc/upgrading.txt | 15 +++ roundup/configuration.py | 95 +++++++++++++- test/test_config.py | 195 +++++++++++++++++++++++++++ 5 files changed, 570 insertions(+), 12 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index b181e066..ac2c5422 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -32,6 +32,8 @@ Features: - add support for authorized changes. User can be prompted to enter their password to authorize a change. If the user's password is properly entered, the change is committed. (John Rouillard) +- add support for dictConfig style logging configuration. Ini/File + style configs will still be supported. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 538ad8fb..42e73e29 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -47,31 +47,284 @@ There's two "installations" that we talk about when using Roundup: in the tracker's config.ini. -Configuring Roundup's Logging of Messages For Sysadmins -======================================================= +Configuring Roundup Message Logging +=================================== -You may configure where Roundup logs messages in your tracker's config.ini -file. Roundup will use the standard Python (2.3+) logging implementation. +You can control how Roundup logs messages using your tracker's +config.ini file. Roundup uses the standard Python (2.3+) logging +implementation. The config file and ``roundup-server`` provide very +basic control over logging. -Configuration for standard "logging" module: - - tracker configuration file specifies the location of a logging - configration file as ``logging`` -> ``config`` - - ``roundup-server`` specifies the location of a logging configuration - file on the command line Configuration for "BasicLogging" implementation: - tracker configuration file specifies the location of a log file ``logging`` -> ``filename`` - tracker configuration file specifies the level to log to as ``logging`` -> ``level`` + - tracker configuration file lets you disable other loggers + (e.g. when running under a wsgi framework) with + ``logging`` -> ``disable_loggers``. - ``roundup-server`` specifies the location of a log file on the command line - - ``roundup-server`` specifies the level to log to on the command line + - ``roundup-server`` enable using the standard python logger with + the tag/channel ``roundup.http`` on the command line + +By supplying a standard log config file in ini or json (dictionary) +format, you get more control over the logs. You can set different +levels for logs (e.g. roundup.hyperdb can be set to WARNING while +other Roundup log channels are set to INFO and roundup.mailgw logs at +DEBUG level). You can also send the logs for roundup.mailgw to syslog, +and other roundup logs go to an automatically rotating log file, or +are submitted to your log aggregator over https. -(``roundup-mailgw`` always logs to the tracker's log file) +Configuration for standard "logging" module: + - tracker configuration file specifies the location of a logging + configuration file as ``logging`` -> ``config``. In both cases, if no logfile is specified then logging will simply be sent to sys.stderr with only logging of ERROR messages. +Standard Logging Setup +---------------------- + +You can specify your log configs in one of two formats: + + * `fileConfig format + `_ + in ini style + * `dictConfig format + `_ + using json with comment support + +The dictConfig allows more control over configuration including +loading your own log handlers and disabling existing handlers. If you +use the fileConfig format, the ``logging`` -> ``disable_loggers`` flag +in the tracker's config is used to enable/disable pre-existing loggers +as there is no way to do this in the logging config file. + +.. _`dictLogConfig`: + +dictConfig Based Logging Config +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +dictConfigs are specified in JSON format with support for comments. +The file name in the tracker's config for the ``logging`` -> ``config`` +setting must end with ``.json`` to choose the correct processing. + +Comments have to be in one of two forms: + +1. A ``#`` with preceding white space is considered a comment and is + stripped from the file before being passed to the json parser. This + is a "block comment". + +2. A ``#`` preceded by at least three + white space characters is stripped from the end of the line before + begin passed to the json parser. This is an "inline comment". + +Other than this the file is a standard json file that matches the +`Configuration dictionary schema +`_ +defined in the Python documentation. + + +Example dictConfig Logging Config +................................. + +Note that this file is not actually JSON format as it include comments. +So you can not use tools that expect JSON (linters, formatters) to +work with it. + +The config below works with the `Waitress wsgi server +`_ configured to use the +roundup.wsgi channel. It also controls the `TransLogger middleware +`_ configured to use +roundup.wsgi.translogger, to produce httpd style combined logs. The +log file is specified relative to the current working directory not +the tracker home. The tracker home is the subdirectory demo under the +current working directory. The commented config is:: + + { + "version": 1, # only supported version + "disable_existing_loggers": false, # keep the wsgi loggers + + "formatters": { + # standard format for Roundup messages + "standard": { + "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" + }, + # used for waitress wsgi server to produce httpd style logs + "http": { + "format": "%(message)s" + } + }, + "handlers": { + # create an access.log style http log file + "access": { + "level": "INFO", + "formatter": "http", + "class": "logging.FileHandler", + "filename": "demo/access.log" + }, + # logging for roundup.* loggers + "roundup": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.FileHandler", + "filename": "demo/roundup.log" + }, + # print to stdout - fall through for other logging + "default": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout" + } + }, + "loggers": { + "": { + "handlers": [ + "default" + ], + "level": "DEBUG", + "propagate": false + }, + # used by roundup.* loggers + "roundup": { + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false # note pytest testing with caplog requires + # this to be true + }, + "roundup.hyperdb": { + "handlers": [ + "roundup" + ], + "level": "INFO", # can be a little noisy use INFO for production + "propagate": false + }, + "roundup.wsgi": { # using the waitress framework + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false + }, + "roundup.wsgi.translogger": { # httpd style logging + "handlers": [ + "access" + ], + "level": "DEBUG", + "propagate": false + }, + "root": { + "handlers": [ + "default" + ], + "level": "DEBUG", + "propagate": false + } + } + } + +fileConfig Based Logging Config +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The file config is an older and more limited method of configuring +logging. It is described by the `Configuration file format +`_ +in the Python documentation. The file name in the tracker's config for +the ``logging`` -> ``config`` setting must end with ``.ini`` to choose +the correct processing. + +Example fileConfig LoggingConfig +................................ + +This is an example .ini used with roundup-server configured to use +``roundup.http`` channel. It also includes some custom logging +qualnames/tags/channels for logging schema/permission detector and +extension output:: + + [loggers] + #other keys: roundup.hyperdb.backend + keys=root,roundup,roundup.http,roundup.hyperdb,actions,schema,extension,detector + + [logger_root] + #also for root where channlel is not set (NOTSET) aka all + level=DEBUG + handlers=rotate + + [logger_roundup] + # logger for all roundup.* not otherwise configured + level=DEBUG + handlers=rotate + qualname=roundup + propagate=0 + + [logger_roundup.http] + level=INFO + handlers=rotate_weblog + qualname=roundup.http + propagate=0 + + [logger_roundup.hyperdb] + level=WARNING + handlers=rotate + qualname=roundup.hyperdb + propagate=0 + + [logger_actions] + level=INFO + handlers=rotate + qualname=actions + propagate=0 + + [logger_detector] + level=INFO + handlers=rotate + qualname=detector + propagate=0 + + [logger_schema] + level=DEBUG + handlers=rotate + qualname=schema + propagate=0 + + [logger_extension] + level=INFO + handlers=rotate + qualname=extension + propagate=0 + + [handlers] + keys=basic,rotate,rotate_weblog + + [handler_basic] + class=StreamHandler + args=(sys.stderr,) + formatter=basic + + [handler_rotate] + class=logging.handlers.RotatingFileHandler + args=('roundup.log','a', 5120000, 2) + formatter=basic + + [handler_rotate_weblog] + class=logging.handlers.RotatingFileHandler + args=('httpd.log','a', 1024000, 2) + formatter=plain + + [formatters] + keys=basic,plain + + [formatter_basic] + format=%(asctime)s %(process)d %(name)s:%(module)s.%(funcName)s,%(levelname)s: %(message)s + datefmt=%Y-%m-%d %H:%M:%S + + [formatter_plain] + format=%(process)d %(message)s + Configuring roundup-server ========================== diff --git a/doc/upgrading.txt b/doc/upgrading.txt index 66faca56..ac919a5b 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -133,6 +133,21 @@ date, text etc.) do not need JavaScript to work. See :ref:`Confirming the User` in the reference manual for details. +Support for dictConfig Logging Configuration (optional) +------------------------------------------------------- + +Roundup's basic log configuration via config.ini has always had the +ability to use an ini style logging configuration to set levels per +log channel, control output file rotation etc. + +With Roundup 2.6 you can use a JSON like file to configure logging +using `dictConfig +`_. The +JSON file format as been enhanced to support comments that are +stripped before being processed by the logging system. + +You can read about the details in the :ref:`admin manual `. + .. index:: Upgrading; 2.4.0 to 2.5.0 Migrating from 2.4.0 to 2.5.0 diff --git a/roundup/configuration.py b/roundup/configuration.py index 421d91bc..51df14bd 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -43,6 +43,16 @@ def __str__(self): return self.args[0] +class LoggingConfigError(ConfigurationError): + def __init__(self, message, **attrs): + super().__init__(message) + for key, value in attrs.items(): + self.__setattr__(key, value) + + def __str__(self): + return self.args[0] + + class NoConfigError(ConfigurationError): """Raised when configuration loading fails @@ -2330,14 +2340,97 @@ def reset(self): self.detectors.reset() self.init_logging() + def load_config_dict_from_json_file(self, filename): + import json + comment_re = re.compile( + r"""^\s*\#.* # comment at beginning of line possibly indented. + | # or + ^(.*)\s\s\s\#.* # comment char preceeded by at least three spaces. + """, re.VERBOSE) + + config_list = [] + with open(filename) as config_file: + for line in config_file: + match = comment_re.search(line) + if match: + if match.lastindex: + config_list.append(match.group(1) + "\n") + else: + # insert blank line for comment line to + # keep line numbers in sync. + config_list.append("\n") + continue + config_list.append(line) + + try: + config_dict = json.loads("".join(config_list)) + except json.decoder.JSONDecodeError as e: + error_at_doc_line = e.lineno + # subtract 1 - zero index on config_list + # remove '\n' for display + line = config_list[error_at_doc_line - 1][:-1] + + hint = "" + if line.find('#') != -1: + hint = "\nMaybe bad inline comment, 3 spaces needed before #." + + raise LoggingConfigError( + 'Error parsing json logging dict (%(file)s) ' + 'near \n\n %(line)s\n\n' + '%(msg)s: line %(lineno)s column %(colno)s.%(hint)s' % + {"file": filename, + "line": line, + "msg": e.msg, + "lineno": error_at_doc_line, + "colno": e.colno, + "hint": hint}, + config_file=self.filepath, + source="json.loads" + ) + + return config_dict + def init_logging(self): _file = self["LOGGING_CONFIG"] - if _file and os.path.isfile(_file): + if _file and os.path.isfile(_file) and _file.endswith(".ini"): logging.config.fileConfig( _file, disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"]) return + if _file and os.path.isfile(_file) and _file.endswith(".json"): + config_dict = self.load_config_dict_from_json_file(_file) + try: + logging.config.dictConfig(config_dict) + except ValueError as e: + # docs say these exceptions: + # ValueError, TypeError, AttributeError, ImportError + # could be raised, but + # looking through the code, it looks like + # configure() maps all exceptions (including + # ImportError, TypeError) raised by functions to + # ValueError. + context = "No additional information available." + if hasattr(e, '__context__') and e.__context__: + # get additional error info. E.G. if INFO + # is replaced by MANGO, context is: + # ValueError("Unknown level: 'MANGO'") + # while str(e) is "Unable to configure handler 'access'" + context = e.__context__ + + raise LoggingConfigError( + 'Error loading logging dict from %(file)s.\n' + '%(msg)s\n%(context)s\n' % { + "file": _file, + "msg": type(e).__name__ + ": " + str(e), + "context": context + }, + config_file=self.filepath, + source="dictConfig" + ) + + return + _file = self["LOGGING_FILENAME"] # set file & level on the roundup logger logger = logging.getLogger('roundup') diff --git a/test/test_config.py b/test/test_config.py index aa3bb968..f7e630a7 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -25,6 +25,7 @@ import unittest from os.path import normpath +from textwrap import dedent from roundup import configuration from roundup.backends import get_backend, have_backend @@ -1046,3 +1047,197 @@ def testInvalidIndexerValue(self): print(string_rep) self.assertIn("nati", string_rep) self.assertIn("'whoosh'", string_rep) + + def testDictLoggerConfigViaJson(self): + + # test case broken, comment on version line misformatted + config1 = dedent(""" + { + "version": 1, # only supported version + "disable_existing_loggers": false, # keep the wsgi loggers + + "formatters": { + # standard Roundup formatter including context id. + "standard": { + "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" + }, + # used for waitress wsgi server to produce httpd style logs + "http": { + "format": "%(message)s" + } + }, + "handlers": { + # create an access.log style http log file + "access": { + "level": "INFO", + "formatter": "http", + "class": "logging.FileHandler", + "filename": "demo/access.log" + }, + # logging for roundup.* loggers + "roundup": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.FileHandler", + "filename": "demo/roundup.log" + }, + # print to stdout - fall through for other logging + "default": { + "level": "DEBUG", + "formatter": "standard", + "class": "logging.StreamHandler", + "stream": "ext://sys.stdout" + } + }, + "loggers": { + "": { + "handlers": [ + "default" # used by wsgi/usgi + ], + "level": "DEBUG", + "propagate": false + }, + # used by roundup.* loggers + "roundup": { + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false # note pytest testing with caplog requires + # this to be true + }, + "roundup.hyperdb": { + "handlers": [ + "roundup" + ], + "level": "INFO", # can be a little noisy INFO for production + "propagate": false + }, + "roundup.wsgi": { # using the waitress framework + "handlers": [ + "roundup" + ], + "level": "DEBUG", + "propagate": false + }, + "roundup.wsgi.translogger": { # httpd style logging + "handlers": [ + "access" + ], + "level": "DEBUG", + "propagate": false + }, + "root": { + "handlers": [ + "default" + ], + "level": "DEBUG", + "propagate": false + } + } + } + """) + + log_config_filename = self.instance.tracker_home \ + + "/_test_log_config.json" + + # happy path + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(config1) + + config = self.db.config.load_config_dict_from_json_file( + log_config_filename) + self.assertIn("version", config) + self.assertEqual(config['version'], 1) + + # broken inline comment misformatted + test_config = config1.replace(": 1, #", ": 1, #") + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.load_config_dict_from_json_file( + log_config_filename) + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n ' + '"version": 1, # only supported version\n\nExpecting ' + 'property name enclosed in double quotes: line 3 column 18.\n' + 'Maybe bad inline comment, 3 spaces needed before #.') + ) + + # broken trailing , on last dict element + test_config = config1.replace(' "ext://sys.stdout"', + ' "ext://sys.stdout",' + ) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.load_config_dict_from_json_file( + log_config_filename) + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n' + ' }\n\nExpecting property name enclosed in double ' + 'quotes: line 37 column 6.') + ) + + # happy path for init_logging() + + # verify preconditions + logger = logging.getLogger("roundup") + self.assertEqual(logger.level, 40) # error default from config.ini + self.assertEqual(logger.filters, []) + + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(config1) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + config = self.db.config.init_logging() + self.assertIs(config, None) + + logger = logging.getLogger("roundup") + self.assertEqual(logger.level, 10) # debug + self.assertEqual(logger.filters, []) + + # broken invalid format type (int not str) + test_config = config1.replace('"format": "%(message)s"', + '"format": 1234',) + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + self.assertEqual( + cm.exception.args[0], + ('Error loading logging dict from ' + '_test_instance/_test_log_config.json.\n' + "ValueError: Unable to configure formatter 'http'\n" + 'expected string or bytes-like object\n') + ) + + # broken invalid level MANGO + test_config = config1.replace( + ': "INFO", # can', + ': "MANGO", # can') + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + self.assertEqual( + cm.exception.args[0], + ("Error loading logging dict from " + "_test_instance/_test_log_config.json.\nValueError: " + "Unable to configure logger 'roundup.hyperdb'\nUnknown level: " + "'MANGO'\n") + + ) From ab9a37b30a0520653e3751a00605dfd6a1fed5e7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 11:17:23 -0400 Subject: [PATCH 05/26] test: fix testDictLoggerConfigViaJson The path to the log file in the config did not exist. Change to use the tracker home. Also add a test where the log file directory does not exist. This reports the full path, so have to edit the full path in the error message before comparison. --- test/test_config.py | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/test/test_config.py b/test/test_config.py index f7e630a7..80c469c5 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -20,6 +20,7 @@ import logging import os import pytest +import re import shutil import sys import unittest @@ -1072,14 +1073,14 @@ def testDictLoggerConfigViaJson(self): "level": "INFO", "formatter": "http", "class": "logging.FileHandler", - "filename": "demo/access.log" + "filename": "_test_instance/access.log" }, # logging for roundup.* loggers "roundup": { "level": "DEBUG", "formatter": "standard", "class": "logging.FileHandler", - "filename": "demo/roundup.log" + "filename": "_test_instance/roundup.log" }, # print to stdout - fall through for other logging "default": { @@ -1241,3 +1242,29 @@ def testDictLoggerConfigViaJson(self): "'MANGO'\n") ) + + # broken invalid output directory + test_config = config1.replace( + ' "_test_instance/access.log"', + ' "not_a_test_instance/access.log"') + with open(log_config_filename, "w") as log_config_file: + log_config_file.write(test_config) + + # file is made relative to tracker dir. + self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # error includes full path which is different on different + # CI and dev platforms. So munge the path using re.sub. + self.assertEqual( + re.sub("directory: \'/.*not_a", 'directory: not_a' , + cm.exception.args[0]), + ("Error loading logging dict from " + "_test_instance/_test_log_config.json.\n" + "ValueError: Unable to configure handler 'access'\n" + "[Errno 2] No such file or directory: " + "not_a_test_instance/access.log'\n" + ) + ) + From d510304e215d551d0be60536a04ff382b83c817e Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 11:23:39 -0400 Subject: [PATCH 06/26] chore: bump actions/checkout as reported by dependabot. --- .github/workflows/anchore.yml | 2 +- .github/workflows/build-xapian.yml | 2 +- .github/workflows/ci-test.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/ossf-scorecard.yml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/anchore.yml b/.github/workflows/anchore.yml index 7eff99f1..3ab5b7cf 100644 --- a/.github/workflows/anchore.yml +++ b/.github/workflows/anchore.yml @@ -37,7 +37,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout the code - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - name: Build the Docker image run: docker pull python:3-alpine; docker build . --file scripts/Docker/Dockerfile --tag localbuild/testimage:latest - name: List the Docker image diff --git a/.github/workflows/build-xapian.yml b/.github/workflows/build-xapian.yml index 4d6edf6d..00e6fe09 100644 --- a/.github/workflows/build-xapian.yml +++ b/.github/workflows/build-xapian.yml @@ -42,7 +42,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 # Setup version of Python to use - name: Set Up Python 3.13 diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index 8e88da76..c0a66398 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -116,7 +116,7 @@ jobs: # if: {{ false }} # continue running if step fails # continue-on-error: true - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 # Setup version of Python to use - name: Set Up Python ${{ matrix.python-version }} diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 0b214cc7..d5a70850 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -49,7 +49,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/ossf-scorecard.yml b/.github/workflows/ossf-scorecard.yml index c6a5547e..28075031 100644 --- a/.github/workflows/ossf-scorecard.yml +++ b/.github/workflows/ossf-scorecard.yml @@ -35,7 +35,7 @@ jobs: steps: - name: "Checkout code" - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: persist-credentials: false From 797154675cdfd8457c19bde09f83dc90d616b8f4 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 12:53:14 -0400 Subject: [PATCH 07/26] test: more fixes for variations among python versions. reset logging. The loggers I installed are bleeding through to tests in other modules. Then handle recogniton of: trailing comma in json reporting line with comma not following line incorrect type value in dictConfig int where string should be 3.7 fils to detect. 3.10+ add "got 'int'" to reason. --- test/test_config.py | 68 +++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/test/test_config.py b/test/test_config.py index 80c469c5..ded20d23 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1051,7 +1051,7 @@ def testInvalidIndexerValue(self): def testDictLoggerConfigViaJson(self): - # test case broken, comment on version line misformatted + # good base test case config1 = dedent(""" { "version": 1, # only supported version @@ -1178,13 +1178,30 @@ def testDictLoggerConfigViaJson(self): with self.assertRaises(configuration.LoggingConfigError) as cm: config = self.db.config.load_config_dict_from_json_file( log_config_filename) - self.assertEqual( - cm.exception.args[0], - ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n' - ' }\n\nExpecting property name enclosed in double ' - 'quotes: line 37 column 6.') - ) + #pre 3.12?? + # FIXME check/remove when 3.13. is min supported version + if "property name" in cm.exception.args[0]: + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n' + ' }\n\nExpecting property name enclosed in double ' + 'quotes: line 37 column 6.') + ) + + # 3.13+ diags FIXME + print('FINDME') + print(cm.exception.args[0]) + _junk = ''' + if "property name" not in cm.exception.args[0]: + self.assertEqual( + cm.exception.args[0], + ('Error parsing json logging dict ' + '(_test_instance/_test_log_config.json) near \n\n' + ' }\n\nExpecting property name enclosed in double ' + 'quotes: line 37 column 6.') + ) + ''' # happy path for init_logging() @@ -1213,15 +1230,25 @@ def testDictLoggerConfigViaJson(self): # file is made relative to tracker dir. self.db.config["LOGGING_CONFIG"] = '_test_log_config.json' - with self.assertRaises(configuration.LoggingConfigError) as cm: - config = self.db.config.init_logging() - self.assertEqual( - cm.exception.args[0], - ('Error loading logging dict from ' - '_test_instance/_test_log_config.json.\n' - "ValueError: Unable to configure formatter 'http'\n" - 'expected string or bytes-like object\n') - ) + + + # different versions of python have different errors + # (or no error for this case in 3.7) + # FIXME remove version check post 3.7 as minimum version + if sys.version_info > (3,7): + with self.assertRaises(configuration.LoggingConfigError) as cm: + config = self.db.config.init_logging() + + # mangle args[0] to add got 'int' + # FIXME: remove mangle after 3.12 min version + self.assertEqual( + cm.exception.args[0].replace( + "object\n", "object, got 'int'\n"), + ('Error loading logging dict from ' + '_test_instance/_test_log_config.json.\n' + "ValueError: Unable to configure formatter 'http'\n" + "expected string or bytes-like object, got 'int'\n") + ) # broken invalid level MANGO test_config = config1.replace( @@ -1268,3 +1295,10 @@ def testDictLoggerConfigViaJson(self): ) ) + # rip down all the loggers leaving the root logger reporting to stdout. + # otherwise logger config is leaking to other tests + + from importlib import reload + logging.shutdown() + reload(logging) + From 24b82e46659ba007e5a588e42a35fa8e4be9e294 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 16:13:52 -0400 Subject: [PATCH 08/26] test: more fun with logger leakage. I have disabled all calls to dictConfig. I can't see how to stop this from leaking. I even tried storing the root and roundup logger state (copying lists) before running anything and restoring afterwards. the funny part is if I removae all dictConfig calls and keep the state saving and logger reset and restoring code it sill fails. It passes if I don't restore the handler state for the root logger. However the test will fail even when I comment out the root logger config in the dict if I apply the dict. ???? --- test/test_config.py | 55 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/test/test_config.py b/test/test_config.py index ded20d23..a7f4a6ee 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1139,6 +1139,20 @@ def testDictLoggerConfigViaJson(self): } """) + # save roundup logger state + loggernames = ("", "roundup") + logger_state = {} + for name in loggernames: + logger_state[name] = {} + + roundup_logger = logging.getLogger("roundup") + for i in ("filters", "handlers", "level", "propagate"): + attr = getattr(roundup_logger, i) + if isinstance(attr, list): + logger_state[name][i] = attr.copy() + else: + logger_state[name][i] = getattr(roundup_logger, i) + log_config_filename = self.instance.tracker_home \ + "/_test_log_config.json" @@ -1203,6 +1217,16 @@ def testDictLoggerConfigViaJson(self): ) ''' + ''' + # comment out as it breaks the logging config for caplog + # on test_rest.py:testBadFormAttributeErrorException + # for all rdbms backends. + # the log ERROR check never gets any info + + # commenting out root logger in config doesn't make it work. + # storing root logger and roundup logger state and restoring it + # still fails. + # happy path for init_logging() # verify preconditions @@ -1295,9 +1319,38 @@ def testDictLoggerConfigViaJson(self): ) ) - # rip down all the loggers leaving the root logger reporting to stdout. + ''' + # rip down all the loggers leaving the root logger reporting + # to stdout. # otherwise logger config is leaking to other tests + roundup_loggers = [logging.getLogger(name) for name in + logging.root.manager.loggerDict + if name.startswith("roundup")] + + # cribbed from configuration.py:init_loggers + hdlr = logging.StreamHandler(sys.stdout) + formatter = logging.Formatter( + '%(asctime)s %(levelname)s %(message)s') + hdlr.setFormatter(formatter) + + for logger in roundup_loggers: + # no logging API to remove all existing handlers!?! + for h in logger.handlers: + h.close() + logger.removeHandler(h) + logger.handlers = [hdlr] + logger.setLevel("DEBUG") + logger.propagate = True + + for name in loggernames: + local_logger = logging.getLogger(name) + for attr in logger_state[name]: + # if I restore handlers state for root logger + # I break the test referenced above. -- WHY???? + if attr == "handlers" and name == "": continue + setattr(local_logger, attr, logger_state[name][attr]) + from importlib import reload logging.shutdown() reload(logging) From bf02f353d19be24dcf8ad7ee6ed2bb2aac7d19ad Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Wed, 20 Aug 2025 21:04:56 -0400 Subject: [PATCH 09/26] test: test dictLoggerConfig - working logging reset and windows Finally figured out why things weren't being restored. Bug in the code. Created a class fixture that stores and restores the logging config. Also using os.path.join and other machinations to make the tests run under windows and linux correctly. --- test/test_config.py | 187 ++++++++++++++++++++++++-------------------- 1 file changed, 101 insertions(+), 86 deletions(-) diff --git a/test/test_config.py b/test/test_config.py index a7f4a6ee..7d1ed541 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -415,8 +415,71 @@ def testOctalNumberOption(self): print(type(config._get_option('UMASK'))) +@pytest.mark.usefixtures("save_restore_logging") class TrackerConfig(unittest.TestCase): + @pytest.fixture(scope="class") + def save_restore_logging(self): + """Save logger state and try to restore it after all tests in + this class have finished. + + The primary test is testDictLoggerConfigViaJson which + can change the loggers and break tests that depend on caplog + """ + # Save logger state for root and roundup top level logger + loggernames = ("", "roundup") + + # The state attributes to save. Lists are shallow copied + state_to_save = ("filters", "handlers", "level", "propagate") + + logger_state = {} + for name in loggernames: + logger_state[name] = {} + roundup_logger = logging.getLogger(name) + + for i in state_to_save: + attr = getattr(roundup_logger, i) + if isinstance(attr, list): + logger_state[name][i] = attr.copy() + else: + logger_state[name][i] = getattr(roundup_logger, i) + + # run all class tests here + yield + + # rip down all the loggers leaving the root logger reporting + # to stdout. + # otherwise logger config is leaking to other tests + roundup_loggers = [logging.getLogger(name) for name in + logging.root.manager.loggerDict + if name.startswith("roundup")] + + # cribbed from configuration.py:init_loggers + hdlr = logging.StreamHandler(sys.stdout) + formatter = logging.Formatter( + '%(asctime)s %(levelname)s %(message)s') + hdlr.setFormatter(formatter) + + for logger in roundup_loggers: + # no logging API to remove all existing handlers!?! + for h in logger.handlers: + h.close() + logger.removeHandler(h) + logger.handlers = [hdlr] + logger.setLevel("WARNING") + logger.propagate = True # important as caplog requires this + + # Restore the info we stored before running tests + for name in loggernames: + local_logger = logging.getLogger(name) + for attr in logger_state[name]: + setattr(local_logger, attr, logger_state[name][attr]) + + # reset logging as well + from importlib import reload + logging.shutdown() + reload(logging) + backend = 'anydbm' def setUp(self): @@ -1139,22 +1202,8 @@ def testDictLoggerConfigViaJson(self): } """) - # save roundup logger state - loggernames = ("", "roundup") - logger_state = {} - for name in loggernames: - logger_state[name] = {} - - roundup_logger = logging.getLogger("roundup") - for i in ("filters", "handlers", "level", "propagate"): - attr = getattr(roundup_logger, i) - if isinstance(attr, list): - logger_state[name][i] = attr.copy() - else: - logger_state[name][i] = getattr(roundup_logger, i) - - log_config_filename = self.instance.tracker_home \ - + "/_test_log_config.json" + log_config_filename = os.path.join(self.instance.tracker_home, + "_test_log_config.json") # happy path with open(log_config_filename, "w") as log_config_file: @@ -1176,10 +1225,11 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n ' + '(%s) near \n\n ' '"version": 1, # only supported version\n\nExpecting ' 'property name enclosed in double quotes: line 3 column 18.\n' - 'Maybe bad inline comment, 3 spaces needed before #.') + 'Maybe bad inline comment, 3 spaces needed before #.' % + log_config_filename) ) # broken trailing , on last dict element @@ -1198,9 +1248,10 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n' - ' }\n\nExpecting property name enclosed in double ' - 'quotes: line 37 column 6.') + '(%s) near \n\n' + ' }\n\n' + 'Expecting property name enclosed in double ' + 'quotes: line 37 column 6.' % log_config_filename) ) # 3.13+ diags FIXME @@ -1211,22 +1262,12 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ('Error parsing json logging dict ' - '(_test_instance/_test_log_config.json) near \n\n' - ' }\n\nExpecting property name enclosed in double ' - 'quotes: line 37 column 6.') + '(%s) near \n\n' + ' "stream": "ext://sys.stdout"\n\n' + 'Expecting property name enclosed in double ' + 'quotes: line 37 column 6.' % log_config_filename) ) ''' - - ''' - # comment out as it breaks the logging config for caplog - # on test_rest.py:testBadFormAttributeErrorException - # for all rdbms backends. - # the log ERROR check never gets any info - - # commenting out root logger in config doesn't make it work. - # storing root logger and roundup logger state and restoring it - # still fails. - # happy path for init_logging() # verify preconditions @@ -1269,9 +1310,10 @@ def testDictLoggerConfigViaJson(self): cm.exception.args[0].replace( "object\n", "object, got 'int'\n"), ('Error loading logging dict from ' - '_test_instance/_test_log_config.json.\n' + '%s.\n' "ValueError: Unable to configure formatter 'http'\n" - "expected string or bytes-like object, got 'int'\n") + "expected string or bytes-like object, got 'int'\n" % + log_config_filename) ) # broken invalid level MANGO @@ -1288,9 +1330,9 @@ def testDictLoggerConfigViaJson(self): self.assertEqual( cm.exception.args[0], ("Error loading logging dict from " - "_test_instance/_test_log_config.json.\nValueError: " + "%s.\nValueError: " "Unable to configure logger 'roundup.hyperdb'\nUnknown level: " - "'MANGO'\n") + "'MANGO'\n" % log_config_filename) ) @@ -1298,6 +1340,8 @@ def testDictLoggerConfigViaJson(self): test_config = config1.replace( ' "_test_instance/access.log"', ' "not_a_test_instance/access.log"') + access_filename = os.path.join("not_a_test_instance", "access.log") + with open(log_config_filename, "w") as log_config_file: log_config_file.write(test_config) @@ -1307,51 +1351,22 @@ def testDictLoggerConfigViaJson(self): config = self.db.config.init_logging() # error includes full path which is different on different - # CI and dev platforms. So munge the path using re.sub. - self.assertEqual( - re.sub("directory: \'/.*not_a", 'directory: not_a' , - cm.exception.args[0]), - ("Error loading logging dict from " - "_test_instance/_test_log_config.json.\n" - "ValueError: Unable to configure handler 'access'\n" - "[Errno 2] No such file or directory: " - "not_a_test_instance/access.log'\n" - ) - ) - - ''' - # rip down all the loggers leaving the root logger reporting - # to stdout. - # otherwise logger config is leaking to other tests - - roundup_loggers = [logging.getLogger(name) for name in - logging.root.manager.loggerDict - if name.startswith("roundup")] - - # cribbed from configuration.py:init_loggers - hdlr = logging.StreamHandler(sys.stdout) - formatter = logging.Formatter( - '%(asctime)s %(levelname)s %(message)s') - hdlr.setFormatter(formatter) - - for logger in roundup_loggers: - # no logging API to remove all existing handlers!?! - for h in logger.handlers: - h.close() - logger.removeHandler(h) - logger.handlers = [hdlr] - logger.setLevel("DEBUG") - logger.propagate = True - - for name in loggernames: - local_logger = logging.getLogger(name) - for attr in logger_state[name]: - # if I restore handlers state for root logger - # I break the test referenced above. -- WHY???? - if attr == "handlers" and name == "": continue - setattr(local_logger, attr, logger_state[name][attr]) - - from importlib import reload - logging.shutdown() - reload(logging) + # CI and dev platforms. So munge the path using re.sub and + # replace. Windows needs replace as the full path for windows + # to the file has '\\\\' not '\\' when taken from __context__. + # E.G. + # ("Error loading logging dict from ' + # '_test_instance\\_test_log_config.json.\nValueError: ' + # "Unable to configure handler 'access'\n[Errno 2] No such file " + # "or directory: " + # "'C:\\\\tracker\\\\path\\\\not_a_test_instance\\\\access.log'\n") + # sigh..... + output = re.sub("directory: \'.*not_a", 'directory: not_a' , + cm.exception.args[0].replace(r'\\','\\')) + target = ("Error loading logging dict from " + "%s.\n" + "ValueError: Unable to configure handler 'access'\n" + "[Errno 2] No such file or directory: " + "%s'\n" % (log_config_filename, access_filename)) + self.assertEqual(output, target) From 4c31c3931097fa9b2927a968f5443d6d81f12bd7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 21 Aug 2025 09:53:32 -0400 Subject: [PATCH 10/26] test: fix code that does not run a test on 3.7 --- test/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_config.py b/test/test_config.py index 7d1ed541..17ed0ba6 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1300,7 +1300,7 @@ def testDictLoggerConfigViaJson(self): # different versions of python have different errors # (or no error for this case in 3.7) # FIXME remove version check post 3.7 as minimum version - if sys.version_info > (3,7): + if sys.version_info >= (3, 8, 0): with self.assertRaises(configuration.LoggingConfigError) as cm: config = self.db.config.init_logging() From 22da3e23c1b0bc1b59169fcf3b97a71e9d9732f2 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 25 Aug 2025 20:30:51 -0400 Subject: [PATCH 11/26] doc: add check for db.tx_Source to Reauth examples Otherwise it triggers on roundup-admin et al --- doc/customizing.txt | 6 ++++++ doc/reference.txt | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/doc/customizing.txt b/doc/customizing.txt index 62faac18..c3aa5fb2 100644 --- a/doc/customizing.txt +++ b/doc/customizing.txt @@ -1836,6 +1836,12 @@ contents:: # the user has confirmed their identity return + if db.tx_Source not in ("web"): + # the user is using rest, xmlrpc, command line, + # email (unlikely) which don't support interactive + # verification + return + # if the password or email are changing, require id confirmation if 'password' in newvalues: raise Reauth('Add an optional message to the user') diff --git a/doc/reference.txt b/doc/reference.txt index 9713ae28..5d26493f 100644 --- a/doc/reference.txt +++ b/doc/reference.txt @@ -1321,6 +1321,12 @@ user to submit each sensitive property separately. For example:: 'at the same time is not allowed. Please ' 'submit two changes.') + if db.tx_Source not in ("web"): + # the user is using rest, xmlrpc, command line, + # email (unlikely) which don't support interactive + # verification + return + if 'password' in newvalues and not hasattr(db, 'reauth_done'): raise Reauth() From 0bc1ea3d4e4668dfa914780168b1afe2591b98b7 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 25 Aug 2025 20:32:14 -0400 Subject: [PATCH 12/26] doc: reformat markdown-note footnote --- doc/upgrading.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/upgrading.txt b/doc/upgrading.txt index ac919a5b..718a0114 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -949,9 +949,9 @@ bug. Splitting the large script into two parts: allows use of ``structure`` on the script with no replaced strings should it be required for your tracker. -.. [#markdown-note] If you are using markdown formatting for your tracker's notes, - the user will see the markdown label rather than the long - (suspicious) URL. You may want to add something like:: +.. [#markdown-note] If you are using markdown formatting for your + tracker's notes, the user will see the markdown label rather than + the long (suspicious) URL. You may want to add something like:: a[href*=\@template]::after { content: ' [' attr(href) ']'; From 98ef0037b4eff7f1254c1397077338c359700f7b Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 25 Aug 2025 20:44:42 -0400 Subject: [PATCH 13/26] doc: add disable saving roundup-admin history file for password changes --- doc/admin_guide.txt | 9 ++++++--- doc/upgrading.txt | 10 +++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 42e73e29..c94ac59b 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -2151,13 +2151,16 @@ you can put the initialise command and password on the command line. But this allows others on the host to see the password (using the ps command). To initialise a tracker non-interactively without exposing the password, create a file (e.g init_tracker) set to mode -600 (so only the owner can read it) with the contents: +600 (so only the owner can read it) with the contents:: initialise admin_password -and feed it to roundup-admin on standard input. E.G. +and feed it to roundup-admin on standard input. E.G.:: - cat init_tracker | roundup-admin -i tracker_dir + cat init_tracker | roundup-admin -i tracker_dir -P history_features=2 + +setting the pragma ``history_features=2`` prevents storing the command +in the user's history file. (for more details see https://issues.roundup-tracker.org/issue2550789.) diff --git a/doc/upgrading.txt b/doc/upgrading.txt index 718a0114..e814a635 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -2188,7 +2188,7 @@ running:: roundup-admin -i table password,id,username Look for lines starting with ``{CRYPT}``. You can reset the user's -password using:: +password using [#history-pragma]_ :: roundup-admin -i roundup> set user16 password=somenewpassword @@ -2199,6 +2199,14 @@ prompt). This prevents the new password from showing up in the output of ps or shell history. The new password will be encrypted using the default encryption method (usually pbkdf2). +.. [#history-pragma] If your version of roundup-admin provides history + support, you should add ``-P history_features=2`` to the command + line or run ``pragma history_features=2`` at the ``roundup>`` + prompt. This will prevent the command line (and password) from being + saved to your history file (usually ``.roundup_admin_history`` in + your user's home directory. You can use ``roundup-admin -i + pragma list`` to see if pragmas are supported. + Enable performance improvement for wsgi mode (optional) ------------------------------------------------------- From 0c1dcc3ed37f8d78631526c218bc2d48a06c944d Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 26 Aug 2025 22:24:00 -0400 Subject: [PATCH 14/26] feat: change comment in dictConfig json file to // from # Emacs json mode at least will properly indent when using // as a comment character and not #. --- doc/admin_guide.txt | 54 ++++++++++++++++++++++------------------ roundup/configuration.py | 8 +++--- test/test_config.py | 38 ++++++++++++++-------------- 3 files changed, 53 insertions(+), 47 deletions(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index c94ac59b..7228023d 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -110,15 +110,18 @@ dictConfigs are specified in JSON format with support for comments. The file name in the tracker's config for the ``logging`` -> ``config`` setting must end with ``.json`` to choose the correct processing. -Comments have to be in one of two forms: +Comments have to be in one of two forms based on javascript line +comments: -1. A ``#`` with preceding white space is considered a comment and is - stripped from the file before being passed to the json parser. This - is a "block comment". +1. A ``//`` possibly indented with whitespace on a line is considereda + a comment and is stripped from the file before being passed to the + json parser. This is a "line comment". -2. A ``#`` preceded by at least three - white space characters is stripped from the end of the line before - begin passed to the json parser. This is an "inline comment". +2. A ``//` with at least three white space characters before it is + stripped from the end of the line before begin passed to the json + parser. This is an "inline comment". + +Block style comments are not supported. Other than this the file is a standard json file that matches the `Configuration dictionary schema @@ -129,9 +132,12 @@ defined in the Python documentation. Example dictConfig Logging Config ................................. -Note that this file is not actually JSON format as it include comments. -So you can not use tools that expect JSON (linters, formatters) to -work with it. +Note that this file is not actually JSON format as it include +comments. However by using javascript style comments, some tools that +expect JSON (editors, linters, formatters) might work with it. A +command like ``sed -e 's#^\s*//.*##' -e 's#\s*\s\s\s//.*##' +logging.json`` can be used to strip comments for programs that need +it. The config below works with the `Waitress wsgi server `_ configured to use the @@ -143,35 +149,35 @@ the tracker home. The tracker home is the subdirectory demo under the current working directory. The commented config is:: { - "version": 1, # only supported version - "disable_existing_loggers": false, # keep the wsgi loggers + "version": 1, // only supported version + "disable_existing_loggers": false, // keep the wsgi loggers "formatters": { - # standard format for Roundup messages + // standard format for Roundup messages "standard": { - "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" + "format": "%(asctime)s %(ctx_id)s %(levelname)s %(name)s:%(module)s %(msg)s" }, - # used for waitress wsgi server to produce httpd style logs + // used for waitress wsgi server to produce httpd style logs "http": { "format": "%(message)s" } }, "handlers": { - # create an access.log style http log file + // create an access.log style http log file "access": { "level": "INFO", "formatter": "http", "class": "logging.FileHandler", "filename": "demo/access.log" }, - # logging for roundup.* loggers + // logging for roundup.* loggers "roundup": { "level": "DEBUG", "formatter": "standard", "class": "logging.FileHandler", "filename": "demo/roundup.log" }, - # print to stdout - fall through for other logging + // print to stdout - fall through for other logging "default": { "level": "DEBUG", "formatter": "standard", @@ -187,30 +193,30 @@ current working directory. The commented config is:: "level": "DEBUG", "propagate": false }, - # used by roundup.* loggers + // used by roundup.* loggers "roundup": { "handlers": [ "roundup" ], "level": "DEBUG", - "propagate": false # note pytest testing with caplog requires - # this to be true + "propagate": false // note pytest testing with caplog requires + // this to be true }, "roundup.hyperdb": { "handlers": [ "roundup" ], - "level": "INFO", # can be a little noisy use INFO for production + "level": "INFO", // can be a little noisy use INFO for production "propagate": false }, - "roundup.wsgi": { # using the waitress framework + "roundup.wsgi": { // using the waitress framework "handlers": [ "roundup" ], "level": "DEBUG", "propagate": false }, - "roundup.wsgi.translogger": { # httpd style logging + "roundup.wsgi.translogger": { // httpd style logging "handlers": [ "access" ], diff --git a/roundup/configuration.py b/roundup/configuration.py index 51df14bd..fe37240f 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2343,9 +2343,9 @@ def reset(self): def load_config_dict_from_json_file(self, filename): import json comment_re = re.compile( - r"""^\s*\#.* # comment at beginning of line possibly indented. + r"""^\s*//#.* # comment at beginning of line possibly indented. | # or - ^(.*)\s\s\s\#.* # comment char preceeded by at least three spaces. + ^(.*)\s\s\s\//.* # comment char preceeded by at least three spaces. """, re.VERBOSE) config_list = [] @@ -2371,8 +2371,8 @@ def load_config_dict_from_json_file(self, filename): line = config_list[error_at_doc_line - 1][:-1] hint = "" - if line.find('#') != -1: - hint = "\nMaybe bad inline comment, 3 spaces needed before #." + if line.find('//') != -1: + hint = "\nMaybe bad inline comment, 3 spaces needed before //." raise LoggingConfigError( 'Error parsing json logging dict (%(file)s) ' diff --git a/test/test_config.py b/test/test_config.py index 17ed0ba6..1c2834c5 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1117,35 +1117,35 @@ def testDictLoggerConfigViaJson(self): # good base test case config1 = dedent(""" { - "version": 1, # only supported version - "disable_existing_loggers": false, # keep the wsgi loggers + "version": 1, // only supported version + "disable_existing_loggers": false, // keep the wsgi loggers "formatters": { - # standard Roundup formatter including context id. + // standard Roundup formatter including context id. "standard": { "format": "%(asctime)s %(levelname)s %(name)s:%(module)s %(msg)s" }, - # used for waitress wsgi server to produce httpd style logs + // used for waitress wsgi server to produce httpd style logs "http": { "format": "%(message)s" } }, "handlers": { - # create an access.log style http log file + // create an access.log style http log file "access": { "level": "INFO", "formatter": "http", "class": "logging.FileHandler", "filename": "_test_instance/access.log" }, - # logging for roundup.* loggers + // logging for roundup.* loggers "roundup": { "level": "DEBUG", "formatter": "standard", "class": "logging.FileHandler", "filename": "_test_instance/roundup.log" }, - # print to stdout - fall through for other logging + // print to stdout - fall through for other logging "default": { "level": "DEBUG", "formatter": "standard", @@ -1156,35 +1156,35 @@ def testDictLoggerConfigViaJson(self): "loggers": { "": { "handlers": [ - "default" # used by wsgi/usgi + "default" // used by wsgi/usgi ], "level": "DEBUG", "propagate": false }, - # used by roundup.* loggers + // used by roundup.* loggers "roundup": { "handlers": [ "roundup" ], "level": "DEBUG", - "propagate": false # note pytest testing with caplog requires - # this to be true + "propagate": false // note pytest testing with caplog requires + // this to be true }, "roundup.hyperdb": { "handlers": [ "roundup" ], - "level": "INFO", # can be a little noisy INFO for production + "level": "INFO", // can be a little noisy INFO for production "propagate": false }, - "roundup.wsgi": { # using the waitress framework + "roundup.wsgi": { // using the waitress framework "handlers": [ "roundup" ], "level": "DEBUG", "propagate": false }, - "roundup.wsgi.translogger": { # httpd style logging + "roundup.wsgi.translogger": { // httpd style logging "handlers": [ "access" ], @@ -1215,7 +1215,7 @@ def testDictLoggerConfigViaJson(self): self.assertEqual(config['version'], 1) # broken inline comment misformatted - test_config = config1.replace(": 1, #", ": 1, #") + test_config = config1.replace(": 1, //", ": 1, //") with open(log_config_filename, "w") as log_config_file: log_config_file.write(test_config) @@ -1226,9 +1226,9 @@ def testDictLoggerConfigViaJson(self): cm.exception.args[0], ('Error parsing json logging dict ' '(%s) near \n\n ' - '"version": 1, # only supported version\n\nExpecting ' + '"version": 1, // only supported version\n\nExpecting ' 'property name enclosed in double quotes: line 3 column 18.\n' - 'Maybe bad inline comment, 3 spaces needed before #.' % + 'Maybe bad inline comment, 3 spaces needed before //.' % log_config_filename) ) @@ -1318,8 +1318,8 @@ def testDictLoggerConfigViaJson(self): # broken invalid level MANGO test_config = config1.replace( - ': "INFO", # can', - ': "MANGO", # can') + ': "INFO", // can', + ': "MANGO", // can') with open(log_config_filename, "w") as log_config_file: log_config_file.write(test_config) From 50a8b0069a7ab16313d90c9dca11cdef1a13d4c0 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 26 Aug 2025 23:06:40 -0400 Subject: [PATCH 15/26] refactor: also error on missing file or invalid extension Refactored the code to reuse check that logging config file is set and that the file exists. Now throws error and exits if file name does not end in .ini or .json. Now throws error if file doesn't exist. Before it would just configure default logging as though file wasn't specified. Added tests for these two cases. --- roundup/configuration.py | 85 +++++++++++++++++++++++----------------- test/test_config.py | 38 ++++++++++++++++++ 2 files changed, 86 insertions(+), 37 deletions(-) diff --git a/roundup/configuration.py b/roundup/configuration.py index fe37240f..d170eeed 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -2392,45 +2392,56 @@ def load_config_dict_from_json_file(self, filename): def init_logging(self): _file = self["LOGGING_CONFIG"] - if _file and os.path.isfile(_file) and _file.endswith(".ini"): - logging.config.fileConfig( - _file, - disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"]) - return - - if _file and os.path.isfile(_file) and _file.endswith(".json"): - config_dict = self.load_config_dict_from_json_file(_file) - try: - logging.config.dictConfig(config_dict) - except ValueError as e: - # docs say these exceptions: - # ValueError, TypeError, AttributeError, ImportError - # could be raised, but - # looking through the code, it looks like - # configure() maps all exceptions (including - # ImportError, TypeError) raised by functions to - # ValueError. - context = "No additional information available." - if hasattr(e, '__context__') and e.__context__: - # get additional error info. E.G. if INFO - # is replaced by MANGO, context is: - # ValueError("Unknown level: 'MANGO'") - # while str(e) is "Unable to configure handler 'access'" - context = e.__context__ - - raise LoggingConfigError( - 'Error loading logging dict from %(file)s.\n' - '%(msg)s\n%(context)s\n' % { - "file": _file, - "msg": type(e).__name__ + ": " + str(e), - "context": context - }, - config_file=self.filepath, - source="dictConfig" - ) - + if _file and os.path.isfile(_file): + if _file.endswith(".ini"): + logging.config.fileConfig( + _file, + disable_existing_loggers=self["LOGGING_DISABLE_LOGGERS"]) + elif _file.endswith(".json"): + config_dict = self.load_config_dict_from_json_file(_file) + try: + logging.config.dictConfig(config_dict) + except ValueError as e: + # docs say these exceptions: + # ValueError, TypeError, AttributeError, ImportError + # could be raised, but + # looking through the code, it looks like + # configure() maps all exceptions (including + # ImportError, TypeError) raised by functions to + # ValueError. + context = "No additional information available." + if hasattr(e, '__context__') and e.__context__: + # get additional error info. E.G. if INFO + # is replaced by MANGO, context is: + # ValueError("Unknown level: 'MANGO'") + # while str(e) is "Unable to configure handler 'access'" + context = e.__context__ + + raise LoggingConfigError( + 'Error loading logging dict from %(file)s.\n' + '%(msg)s\n%(context)s\n' % { + "file": _file, + "msg": type(e).__name__ + ": " + str(e), + "context": context + }, + config_file=self.filepath, + source="dictConfig" + ) + else: + raise OptionValueError( + self.options['LOGGING_CONFIG'], + _file, + "Unable to load logging config file. " + "File extension must be '.ini' or '.json'.\n" + ) + return + if _file: + raise OptionValueError(self.options['LOGGING_CONFIG'], + _file, + "Unable to find logging config file.") + _file = self["LOGGING_FILENAME"] # set file & level on the roundup logger logger = logging.getLogger('roundup') diff --git a/test/test_config.py b/test/test_config.py index 1c2834c5..130c9c41 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1370,3 +1370,41 @@ def testDictLoggerConfigViaJson(self): "%s'\n" % (log_config_filename, access_filename)) self.assertEqual(output, target) + def test_missing_logging_config_file(self): + saved_config = self.db.config['LOGGING_CONFIG'] + + self.db.config['LOGGING_CONFIG'] = 'logging.json' + + with self.assertRaises(configuration.OptionValueError) as cm: + self.db.config.init_logging() + + self.assertEqual(cm.exception.args[1], "_test_instance/logging.json") + self.assertEqual(cm.exception.args[2], + "Unable to find logging config file.") + + self.db.config['LOGGING_CONFIG'] = 'logging.ini' + + with self.assertRaises(configuration.OptionValueError) as cm: + self.db.config.init_logging() + + self.assertEqual(cm.exception.args[1], "_test_instance/logging.ini") + self.assertEqual(cm.exception.args[2], + "Unable to find logging config file.") + + self.db.config['LOGGING_CONFIG'] = saved_config + + def test_unknown_logging_config_file_type(self): + saved_config = self.db.config['LOGGING_CONFIG'] + + self.db.config['LOGGING_CONFIG'] = 'schema.py' + + + with self.assertRaises(configuration.OptionValueError) as cm: + self.db.config.init_logging() + + self.assertEqual(cm.exception.args[1], "_test_instance/schema.py") + self.assertEqual(cm.exception.args[2], + "Unable to load logging config file. " + "File extension must be '.ini' or '.json'.\n") + + self.db.config['LOGGING_CONFIG'] = saved_config From 0c3458bac0cbd3cab34cfb3a9d7e9feee91c34ad Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Tue, 26 Aug 2025 23:37:42 -0400 Subject: [PATCH 16/26] feat: add 'q' as alias to quit to exit interactive roundup-admin Also require no arguments to 'q', 'quit' or 'exit' before exiting. Now typing 'quit a' will get an unknown command error. Add to admin-guide how to get out of interactive mode. Also test 'q' and 'exit' commands. No upgrading docs added. Not that big a feature. Just noted in CHANGES. Reporting error if argument provided is unlikely to be an issue IMO, so no upgrading.txt entry. --- CHANGES.txt | 2 ++ doc/admin_guide.txt | 3 +++ roundup/admin.py | 3 ++- test/test_admin.py | 4 ++-- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index ac2c5422..f3d7cea3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,6 +34,8 @@ Features: properly entered, the change is committed. (John Rouillard) - add support for dictConfig style logging configuration. Ini/File style configs will still be supported. (John Rouillard) +- add 'q' as alias for quit in roundup-admin interactive mode. (John + Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index 7228023d..bcd5101f 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -2067,6 +2067,9 @@ The basic usage is: Commands may be abbreviated as long as the abbreviation matches only one command, e.g. l == li == lis == list. +In interactive mode entering: ``q``, ``quit``, or ``exit`` alone on a +line will exit the program. + One thing to note, The ``-u user`` setting does not currently operate like a user logging in via the web. The user running roundup-admin must have read access to the tracker home directory. As a result the diff --git a/roundup/admin.py b/roundup/admin.py index 5fd1394b..f52aaa26 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -2415,7 +2415,8 @@ def interactive(self): except ValueError: continue # Ignore invalid quoted token if not args: continue # noqa: E701 - if args[0] in ('quit', 'exit'): break # noqa: E701 + if args[0] in ('q', 'quit', 'exit') and len(args) == 1: + break # noqa: E701 self.run_command(args) # exit.. check for transactions diff --git a/test/test_admin.py b/test/test_admin.py index e6387098..97018453 100644 --- a/test/test_admin.py +++ b/test/test_admin.py @@ -150,7 +150,7 @@ def testBasicInteractive(self): expected = 'ready for input.\nType "help" for help.' self.assertEqual(expected, out[-1*len(expected):]) - inputs = iter(["list user", "quit"]) + inputs = iter(["list user", "q"]) AdminTool.my_input = lambda _self, _prompt: next(inputs) @@ -1067,7 +1067,7 @@ def testPragma_reopen_tracker(self): # must set verbose to see _reopen_tracker hidden setting. # and to get "Reopening tracker" verbose log output - inputs = iter(["pragma verbose=true", "pragma list", "quit"]) + inputs = iter(["pragma verbose=true", "pragma list", "exit"]) AdminTool.my_input = lambda _self, _prompt: next(inputs) self.install_init() From 87adb70dca81d4f2e0e4a538799fa825638a2d7b Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 28 Aug 2025 11:13:24 -0400 Subject: [PATCH 17/26] build(chore): update bump codecov/codecov-action from 5.4.3 to 5.5.0 https://github.com/roundup-tracker/roundup/pull/61 by dependabot --- .github/workflows/ci-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index c0a66398..35a5a948 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -323,7 +323,7 @@ jobs: - name: Upload coverage to Codecov # see: https://github.com/codecov/codecov-action#usage - uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 + uses: codecov/codecov-action@fdcc8476540edceab3de004e990f80d881c6cc00 # v5.5.0 with: verbose: true token: ${{ secrets.CODECOV_TOKEN }} From 8547740c034d54225f47d507b9d76c7d551c270d Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 28 Aug 2025 11:30:11 -0400 Subject: [PATCH 18/26] chore: update sha256 index for latest pyhton:3-alpine image. --- scripts/Docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Docker/Dockerfile b/scripts/Docker/Dockerfile index 7ab53982..b3424cd1 100644 --- a/scripts/Docker/Dockerfile +++ b/scripts/Docker/Dockerfile @@ -26,7 +26,7 @@ ARG source=local # Note this is the index digest for the image, not the manifest digest. # The index digest is shared across archetectures (amd64, arm64 etc.) # while the manifest digest is unique per platform/arch. -ARG SHA256=9b4929a72599b6c6389ece4ecbf415fd1355129f22bb92bb137eea098f05e975 +ARG SHA256=9ba6d8cbebf0fb6546ae71f2a1c14f6ffd2fdab83af7fa5669734ef30ad48844 # Set to any non-empty value to enable shell debugging for troubleshooting ARG VERBOSE= From 2bdcc6b84772a00373fded524452760bfb299087 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Thu, 28 Aug 2025 12:39:38 -0400 Subject: [PATCH 19/26] test - fix parsing of integer param values CI broke on the string '1\r#' expecting a 400 but got a 200 in test_element_url_param_accepting_integer_values(). The #, & characters mark a url fragment or start of another parameter and not part of the value. In a couple of tests, I parse the hypothesis generated value to remove a # or & and anything after. Then I set the value to the preceding string. If the string starts with # or &, the value is set to "0" as the server ignores the parameter and returns 200. "0" is a value that asserts that status is 200. The code doing this parsing was different (and broken) between test_element_url_param_accepting_integer_values and test_class_url_param_accepting_integer_values It's now consistent and if it finds a & or #, it actually tests the resulting value/status rather than skipping the test. --- test/test_liveserver.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/test/test_liveserver.py b/test/test_liveserver.py index 4bec62b2..884439b0 100644 --- a/test/test_liveserver.py +++ b/test/test_liveserver.py @@ -256,6 +256,7 @@ class FuzzGetUrls(WsgiSetup, ClientSetup): @given(sampled_from(['@verbose', '@page_size', '@page_index']), text(min_size=1)) + @example("@verbose", "0\r#") @example("@verbose", "1#") @example("@verbose", "#1stuff") @example("@verbose", "0 #stuff") @@ -271,10 +272,18 @@ def test_class_url_param_accepting_integer_values(self, param, value): f = session.get(url, params=query) try: # test case '0 #', '0#', '12345#stuff' '12345&stuff' - match = re.match(r'(^[0-9]*\s*)[#&]', value) + # Normalize like a server does by breaking value at + # # or & as these mark a fragment or subsequent + # query arg and are not part of the value. + match = re.match(r'^(.*)[#&]', value) if match is not None: value = match[1] - elif int(value) >= 0: + # parameter is ignored by server if empty. + # so set it to 0 to force 200 status code. + if value == "": + value = "0" + + if int(value) >= 0: self.assertEqual(f.status_code, 200) except ValueError: # test case '#' '#0', '&', '&anything here really' @@ -285,6 +294,7 @@ def test_class_url_param_accepting_integer_values(self, param, value): self.assertEqual(f.status_code, 400) @given(sampled_from(['@verbose']), text(min_size=1)) + @example("@verbose", "0\r#") @example("@verbose", "10#") @example("@verbose", u'Ø\U000dd990') @settings(max_examples=_max_examples, @@ -298,10 +308,18 @@ def test_element_url_param_accepting_integer_values(self, param, value): f = session.get(url, params=query) try: # test case '0#' '12345#stuff' '12345&stuff' - match = re.match('(^[0-9]*)[#&]', value) + # Normalize like a server does by breaking value at + # # or & as these mark a fragment or subsequent + # query arg and are not part of the value. + match = re.match(r'^(.*)[#&]', value) if match is not None: value = match[1] - elif int(value) >= 0: + # parameter is ignored by server if empty. + # so set it to 0 to force 200 status code. + if value == "": + value = "0" + + if int(value) >= 0: self.assertEqual(f.status_code, 200) except ValueError: # test case '#' '#0', '&', '&anything here really' From c53b5cd6602edf1996a917f06bd5794668f566e8 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 31 Aug 2025 16:54:17 -0400 Subject: [PATCH 20/26] feat: add support for ! history and readline command in roundup-admin Ad support to change input mode emacs/vi using new 'readline' roundup-admin command. Also bind keys to command/input strings, List numbered history and allow rerunning a command with ! or allow user to edit it using !:p. admin_guide.txt: Added docs. admin.py: add functionality. Reconcile import commands to standard. Replace IOError with FileNotFoundError no that we have removed python 2.7 support. Add support for identifying backend used to supply line editing/history functions. Add support for saving commands sent on stdin to history to allow preloading of history. test_admin.py: Test code. Can't test mode changes as lack of pty when driving command line turns off line editing in readline/pyreadline3. Similarly can't test key bindings/settings. Some refactoring of test conditions that had to change because of additional output reporting backend library. --- CHANGES.txt | 3 + doc/admin_guide.txt | 51 +++++++ roundup/admin.py | 205 ++++++++++++++++++++++++-- test/test_admin.py | 350 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 586 insertions(+), 23 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index f3d7cea3..645983f5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -36,6 +36,9 @@ Features: style configs will still be supported. (John Rouillard) - add 'q' as alias for quit in roundup-admin interactive mode. (John Rouillard) +- add readline command to roundup-admin to list history, control input + mode etc. Also support bang (!) commands to rerun commands in history + or put them in the input buffer for editing. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index bcd5101f..c08fee98 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -2202,6 +2202,57 @@ https://pythonhosted.org/pyreadline/usage.html#configuration-file. History is saved to the file ``.roundup_admin_history`` in your home directory (for windows usually ``\Users\``. +In Roundup 2.6.0 and newer, you can use the ``readline`` command to +make changes on the fly. + +* ``readline vi`` - change input mode to use vi key binding when + editing. It starts in entry mode. +* ``readline emacs`` - change input mode to emacs key bindings when + editing. This is also the default. +* ``readline reload`` - reloads the ``~/.roundup_admin_rlrc`` file so + you can test and use changes. +* ``readline history`` - dumps the history buffer and numbers all + commands. +* ``readline .inputrc_command_line`` can be used to make on the fly + key and key sequence bindings to readline commands. It can also be + used to change the internal readline settings using a set + command. For example:: + + readline set bell-style none + + will turn off a ``visible`` or ``audible`` bell. Single character + keybindings:: + + readline Control-o: dump-variables + + to list all the variables that can be set are supported. As are + multi-character bindings:: + + readline "\C-o1": "commit" + + will put "commit" on the input line when you type Control-o followed + by 1. See the `readline manual for details + `_ + on the command lines that can be used. + +Also a limited form of ``!`` (bang) history reference was added. The +reference must be at the start of the line. Typing ``!23`` will rerun +command number 23 from your history. + +Typing ``!23:p`` will load command 23 into the buffer so you can edit +and submit it. Using the bang feature will append the command to the +end of the history list. + +Pyreadline3 users can use ``readline history`` and the +bang commands (including ``:p``). Single character bindings can be +done. For example:: + + readline Control-w: history-search-backward + +The commands that are available are limited compared to Unix's +readline or libedit. Setting variables or entry mode (emacs, +vi) switching do not work in testing. + Using with the shell -------------------- diff --git a/roundup/admin.py b/roundup/admin.py index f52aaa26..2778c6b7 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -34,8 +34,8 @@ import roundup.instance from roundup import __version__ as roundup_version -from roundup import date, hyperdb, init, password, token_r -from roundup.anypy import scandir_ +from roundup import date, hyperdb, init, password, support, token_r +from roundup.anypy import scandir_ # noqa: F401 define os.scandir from roundup.anypy.my_input import my_input from roundup.anypy.strings import repr_export from roundup.configuration import ( @@ -49,7 +49,6 @@ ) from roundup.exceptions import UsageError from roundup.i18n import _, get_translation -from roundup import support try: from UserDict import UserDict @@ -93,6 +92,13 @@ class AdminTool: Additional help may be supplied by help_*() methods. """ + # import here to make AdminTool.readline accessible or + # mockable from tests. + try: + import readline # noqa: I001, PLC0415 + except ImportError: + readline = None + # Make my_input a property to allow overriding in testing. # my_input is imported in other places, so just set it from # the imported value rather than moving def here. @@ -1815,6 +1821,111 @@ def do_pragma(self, args): type(self.settings[setting]).__name__) self.settings[setting] = value + def do_readline(self, args): + ''"""Usage: readline initrc_line | 'emacs' | 'history' | 'reload' | 'vi' + + Using 'reload' will reload the file ~/.roundup_admin_rlrc. + 'history' will show (and number) all commands in the history. + + You can change input mode using the 'emacs' or 'vi' parameters. + The default is emacs. This is the same as using:: + + readline set editing-mode emacs + + or:: + + readline set editing-mode vi + + Any command that can be placed in a readline .inputrc file can + be executed using the readline command. You can assign + dump-variables to control O using:: + + readline Control-o: dump-variables + + Assigning multi-key values also works. + + pyreadline3 support on windows: + + Mode switching doesn't work, emacs only. + + Binding single key commands works with:: + + readline Control-w: history-search-backward + + Multiple key sequences don't work. + + Setting values may work. Difficult to tell because the library + has no way to view the live settings. + + """ + + # TODO: allow history 20 # most recent 20 commands + # history 100-200 # show commands 100-200 + + if not self.readline: + print(_("Readline support is not available.")) + return + # The if test allows pyreadline3 settings like: + # bind_exit_key("Control-z") get through to + # parse_and_bind(). It is not obvious that this form of + # command is supported. Pyreadline3 is supposed to parse + # readline style commands, so we use those for emacs/vi. + # Trying set-mode(...) as in the pyreadline3 init file + # didn't work in testing. + + if len(args) == 1 and args[0].find('(') == -1: + if args[0] == "vi": + self.readline.parse_and_bind("set editing-mode vi") + print(_("Enabled vi mode.")) + elif args[0] == "emacs": + self.readline.parse_and_bind("set editing-mode emacs") + print(_("Enabled emacs mode.")) + elif args[0] == "history": + print("history size", + self.readline.get_current_history_length()) + print('\n'.join([ + str("%3d " % (i + 1) + + self.readline.get_history_item(i + 1)) + for i in range( + self.readline.get_current_history_length()) + ])) + elif args[0] == "reload": + try: + # readline is a singleton. In testing previous + # tests using read_init_file are loading from ~ + # not the test directory because it doesn't + # matter. But for reload we want to test with the + # init file under the test directory. Calling + # read_init_file() calls with the ~/.. init + # location and I can't seem to reset it + # or the readline state. + # So call with explicit file here. + self.readline.read_init_file( + self.get_readline_init_file()) + except FileNotFoundError as e: + # If user invoked reload explicitly, report + # if file not found. + # + # DOES NOT WORK with pyreadline3. Exception + # is not raised if file is missing. + # + # Also e.filename is None under cygwin. A + # simple test case does set e.filename + # correctly?? sigh. So I just call + # get_readline_init_file again to get + # filename. + fn = e.filename or self.get_readline_init_file() + print(_("Init file %s not found.") % fn) + else: + print(_("File %s reloaded.") % + self.get_readline_init_file()) + else: + print(_("Unknown readline parameter %s") % args[0]) + return + + self.readline.parse_and_bind(" ".join(args)) + return + designator_re = re.compile('([A-Za-z]+)([0-9]+)$') designator_rng = re.compile('([A-Za-z]+):([0-9]+)-([0-9]+)$') @@ -2365,29 +2476,34 @@ def history_features(self, feature): # setting the bit disables the feature, so use not. return not self.settings['history_features'] & features[feature] + def get_readline_init_file(self): + return os.path.join(os.path.expanduser("~"), + ".roundup_admin_rlrc") + def interactive(self): """Run in an interactive mode """ print(_('Roundup %s ready for input.\nType "help" for help.') % roundup_version) - initfile = os.path.join(os.path.expanduser("~"), - ".roundup_admin_rlrc") + initfile = self.get_readline_init_file() histfile = os.path.join(os.path.expanduser("~"), ".roundup_admin_history") - try: - import readline + if self.readline: + # clear any history that might be left over from caller + # when reusing AdminTool from tests or program. + self.readline.clear_history() try: if self.history_features('load_rc'): - readline.read_init_file(initfile) - except IOError: # FileNotFoundError under python3 + self.readline.read_init_file(initfile) + except FileNotFoundError: # file is optional pass try: if self.history_features('load_history'): - readline.read_history_file(histfile) + self.readline.read_history_file(histfile) except IOError: # FileNotFoundError under python3 # no history file yet pass @@ -2397,19 +2513,75 @@ def interactive(self): # Pragma history_length allows setting on a per # invocation basis at startup if self.settings['history_length'] != -1: - readline.set_history_length( + self.readline.set_history_length( self.settings['history_length']) - except ImportError: - readline = None - print(_('Note: command history and editing not available')) + if hasattr(self.readline, 'backend'): + # FIXME after min 3.13 version; no backend prints pyreadline3 + print(_("Readline enabled using %s.") % self.readline.backend) + else: + print(_("Readline enabled using unknown library.")) + + else: + print(_('Command history and line editing not available')) + + autosave_enabled = sys.stdin.isatty() and sys.stdout.isatty() while 1: try: command = self.my_input('roundup> ') + # clear an input hook in case it was used to prefill + # buffer. + self.readline.set_pre_input_hook() except EOFError: print(_('exit...')) break if not command: continue # noqa: E701 + if command.startswith('!'): # Pull numbered command from history + print_only = command.endswith(":p") + try: + hist_num = int(command[1:]) \ + if not print_only else int(command[1:-2]) + command = self.readline.get_history_item(hist_num) + except ValueError: + # pass the unknown command + pass + else: + if autosave_enabled and \ + hasattr(self.readline, "replace_history_item"): + # history has the !23 input. Replace it if possible. + # replace_history_item not supported by pyreadline3 + # so !23 will show up in history not the command. + self.readline.replace_history_item( + self.readline.get_current_history_length() - 1, + command) + + if print_only: + # fill the edit buffer with the command + # the user selected. + + # from https://stackoverflow.com/questions/8505163/is-it-possible-to-prefill-a-input-in-python-3s-command-line-interface + # This triggers: + # B023 Function definition does not bind loop variable + # `command` + # in ruff. command will be the value of the command + # variable at the time the function is run. + # Not the value at define time. This is ok since + # hook is run before command is changed by the + # return from (readline) input. + def hook(): + self.readline.insert_text(command) # noqa: B023 + self.readline.redisplay() + self.readline.set_pre_input_hook(hook) + # we clear the hook after the next line is read. + continue + + if not autosave_enabled: + # needed to make testing work and also capture + # commands received on stdin from file/other command + # output. Disable saving with pragma on command line: + # -P history_features=2. + self.readline.add_history(command) + try: args = token_r.token_split(command) except ValueError: @@ -2426,8 +2598,9 @@ def interactive(self): self.db.commit() # looks like histfile is saved with mode 600 - if readline and self.history_features('save_history'): - readline.write_history_file(histfile) + if self.readline and self.history_features('save_history'): + self.readline.write_history_file(histfile) + return 0 def main(self): # noqa: PLR0912, PLR0911 diff --git a/test/test_admin.py b/test/test_admin.py index 97018453..9380115d 100644 --- a/test/test_admin.py +++ b/test/test_admin.py @@ -5,8 +5,17 @@ # from __future__ import print_function +import difflib +import errno import fileinput -import unittest, os, shutil, errno, sys, difflib, re +import io +import os +import platform +import pytest +import re +import shutil +import sys +import unittest from roundup.admin import AdminTool @@ -82,6 +91,10 @@ class AdminTest(object): def setUp(self): self.dirname = '_test_admin' + @pytest.fixture(autouse=True) + def inject_fixtures(self, monkeypatch): + self._monkeypatch = monkeypatch + def tearDown(self): try: shutil.rmtree(self.dirname) @@ -148,7 +161,9 @@ def testBasicInteractive(self): print(ret) self.assertTrue(ret == 0) expected = 'ready for input.\nType "help" for help.' - self.assertEqual(expected, out[-1*len(expected):]) + # back up by 30 to make sure 'ready for input' in slice. + self.assertIn(expected, + "\n".join(out.split('\n')[-3:-1])) inputs = iter(["list user", "q"]) @@ -161,8 +176,10 @@ def testBasicInteractive(self): print(ret) self.assertTrue(ret == 0) - expected = 'help.\n 1: admin\n 2: anonymous' - self.assertEqual(expected, out[-1*len(expected):]) + expected = ' 1: admin\n 2: anonymous' + + self.assertEqual(expected, + "\n".join(out.split('\n')[-2:])) AdminTool.my_input = orig_input @@ -1104,7 +1121,7 @@ def testPragma_reopen_tracker(self): print(ret) self.assertTrue(ret == 0) - self.assertEqual('Reopening tracker', out[2]) + self.assertEqual('Reopening tracker', out[3]) expected = ' _reopen_tracker=True' self.assertIn(expected, out) @@ -1133,7 +1150,7 @@ def testPragma(self): ret = self.admin.main() out = out.getvalue().strip().split('\n') - + print(ret) self.assertTrue(ret == 0) expected = ' verbose=True' @@ -1155,7 +1172,7 @@ def testPragma(self): ret = self.admin.main() out = out.getvalue().strip().split('\n') - + print(ret) self.assertTrue(ret == 0) expected = ' verbose=False' @@ -1810,6 +1827,325 @@ def testSetOnClass(self): self.assertEqual(out, expected) self.assertEqual(len(err), 0) + def testReadline(self): + ''' Note the tests will fail if you run this under pdb. + the context managers capture the pdb prompts and this screws + up the stdout strings with (pdb) prefixed to the line. + ''' + + '''history didn't work when testing. The commands being + executed aren't being sent into the history + buffer. Failed under both windows and linux. + + Explicitly using: readline.set_auto_history(True) in + roundup-admin setup had no effect. + + Looks like monkeypatching stdin is the issue since: + + printf... | roundup-admin | tee + + doesn't work either when printf uses + + "readline vi\nreadline emacs\nreadline history\nquit\n" + + Added explicit readline.add_history() if stdin or + stdout are not a tty to admin.py:interactive(). + + Still no way to drive editing with control/escape + chars to verify editing mode, check keybindings. Need + to trick Admintool to believe it's running on a + tty/pty/con in linux/windows to remove my hack. + ''' + + # Put the init file in the tracker test directory so + # we don't clobber user's actual init file. + original_home = None + if 'HOME' in os.environ: + original_home = os.environ['HOME'] + os.environ['HOME'] = self.dirname + + # same but for windows. + original_userprofile = None + if 'USERPROFILE' in os.environ: + # windows + original_userprofile = os.environ['USERPROFILE'] + os.environ['USERPROFILE'] = self.dirname + + inputs = ["readline vi", "readline emacs", "readline reload", "quit"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + + # verify correct init file is being + self.assertIn(os.path.join(os.path.expanduser("~"), + ".roundup_admin_rlrc"), + self.admin.get_readline_init_file()) + + # No exception is raised for missing file + # under pyreadline3. Detect pyreadline3 looking for: + # readline.Readline + pyreadline = hasattr(self.admin.readline, "Readline") + + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> Enabled vi mode.' + self.assertIn(expected, out) + + expected = 'roundup> Enabled emacs mode.' + self.assertIn(expected, out) + + if not pyreadline: + expected = ('roundup> Init file %s ' + 'not found.' % self.admin.get_readline_init_file()) + self.assertIn(expected, out) + + # --- test 2 + + inputs = ["readline reload", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + with open(self.admin.get_readline_init_file(), + "w") as config_file: + # there is no config line that works for all + # pyreadline3 (windows), readline(*nix), or editline + # (mac). So write empty file. + config_file.write("") + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = ('roundup> File %s reloaded.' % + self.admin.get_readline_init_file()) + + self.assertIn(expected, out) + + # === cleanup + if original_home: + os.environ['HOME'] = original_home + if original_userprofile: + os.environ['USERPROFILE'] = original_userprofile + + def test_admin_history_save_load(self): + # To prevent overwriting/reading user's actual history, + # change HOME enviroment var. + original_home = None + if 'HOME' in os.environ: + original_home = os.environ['HOME'] + os.environ['HOME'] = self.dirname + os.environ['HOME'] = self.dirname + + # same idea but windows + original_userprofile = None + if 'USERPROFILE' in os.environ: + # windows + original_userprofile = os.environ['USERPROFILE'] + os.environ['USERPROFILE'] = self.dirname + + # -- history test + inputs = ["readline history", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # use defaults load/save history + self.admin.settings['history_features'] = 0 + + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> history size 1' + self.assertIn(expected, out) + + expected = ' 1 readline history' + self.assertIn(expected, out) + + # -- history test 3 reruns readline vi + inputs = ["readline vi", "readline history", "!3", + "readline history", "!23s", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + # preserve directory self.install_init() + self.admin=AdminTool() + + # default use all features + #self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + # 4 includes 2 commands in saved history + expected = 'roundup> history size 4' + self.assertIn(expected, out) + + expected = ' 4 readline history' + self.assertIn(expected, out) + + # Shouldn't work on windows. + if platform.system() != "Windows": + expected = ' 5 readline vi' + self.assertIn(expected, out) + else: + # PYREADLINE UNDER WINDOWS + # py3readline on windows can't replace + # command strings in history when connected + # to a console. (Console triggers autosave and + # I have to turn !3 into it's substituted value.) + # but in testing autosave is disabled so + # I don't get the !number but the actual command + # It should have + # + # expected = ' 5 !3' + # + # but it is the same as the unix case. + expected = ' 5 readline vi' + self.assertIn(expected, out) + + expected = ('roundup> Unknown command "!23s" ("help commands" ' + 'for a list)') + self.assertIn(expected, out) + + print(out) + # can't test !#:p mode as readline editing doesn't work + # if not in a tty. + + # === cleanup + if original_home: + os.environ['HOME'] = original_home + if original_userprofile: + os.environ['USERPROFILE'] = original_userprofile + + def test_admin_readline_history(self): + original_home = os.environ['HOME'] + # To prevent overwriting/reading user's actual history, + # change HOME enviroment var. + os.environ['HOME'] = self.dirname + + original_userprofile = None + if 'USERPROFILE' in os.environ: + # windows + original_userprofile = os.environ['USERPROFILE'] + os.environ['USERPROFILE'] = self.dirname + + # -- history test + inputs = ["readline history", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading, but save history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> history size 1' + self.assertIn(expected, out) + + expected = ' 1 readline history' + self.assertIn(expected, out) + + # -- history test + inputs = ["readline vi", "readline history", "!1", "!2", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading, but save history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = 'roundup> history size 2' + self.assertIn(expected, out) + + expected = ' 2 readline history' + self.assertIn(expected, out) + + # doesn't work on windows. + if platform.system() != "Windows": + expected = ' 4 readline history' + self.assertIn(expected, out) + else: + # See + # PYREADLINE UNDER WINDOWS + # elsewhere in this file for why I am not checking for + # expected = ' 4 !2' + expected = ' 4 readline history' + self.assertIn(expected, out) + + # can't test !#:p mode as readline editing doesn't work + # if not in a tty. + + # === cleanup + os.environ['HOME'] = original_home + if original_userprofile: + os.environ['USERPROFILE'] = original_userprofile + def testSpecification(self): ''' Note the tests will fail if you run this under pdb. the context managers capture the pdb prompts and this screws From 55857ec2b20495bd3f3940658f4df13311f3b2db Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Sun, 31 Aug 2025 20:59:04 -0400 Subject: [PATCH 21/26] bug, refactor, test: make pragma history_length work interactively history_length could be set interactively, but it was never used to set readline/pyreadline3's internal state. Using the pragma setting on the roundup-admin command line did set readline's state. Also refactored 2 calls to self.readline.get_current_history_length() into one call and storing in a variable. Also changed method for creating history strings for printing. Tests added for history_length pragma on cli and interactive use. Added test for exiting roundup-admin with EOF on input. Added test for 'readline nosuchdirective' error case. Added test to readline with a command directive to set an internal variable. This last one has no real test to see if it was successful because I can't emulate a real keyboard/tty which is needed to test. --- CHANGES.txt | 3 + roundup/admin.py | 14 +++-- test/test_admin.py | 139 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 5 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 645983f5..fb3b06d3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -26,6 +26,9 @@ Fixed: - cleaned up repo. Close obsolete branches and close a split head due to an identical merge intwo different workicng copies. (John Rouillard) +- in roundup-admin, using 'pragma history_length interactively now + sets readline history length. Using -P history_length=10 on the + command line always worked. (John Rouillard) Features: diff --git a/roundup/admin.py b/roundup/admin.py index 2778c6b7..eb717b73 100644 --- a/roundup/admin.py +++ b/roundup/admin.py @@ -1821,6 +1821,11 @@ def do_pragma(self, args): type(self.settings[setting]).__name__) self.settings[setting] = value + # history_length has to be pushed to readline to have any effect. + if setting == "history_length": + self.readline.set_history_length( + self.settings['history_length']) + def do_readline(self, args): ''"""Usage: readline initrc_line | 'emacs' | 'history' | 'reload' | 'vi' @@ -1881,13 +1886,12 @@ def do_readline(self, args): self.readline.parse_and_bind("set editing-mode emacs") print(_("Enabled emacs mode.")) elif args[0] == "history": - print("history size", - self.readline.get_current_history_length()) + history_size = self.readline.get_current_history_length() + print("history size", history_size) print('\n'.join([ - str("%3d " % (i + 1) + + "%3d %s" % ((i + 1), self.readline.get_history_item(i + 1)) - for i in range( - self.readline.get_current_history_length()) + for i in range(history_size) ])) elif args[0] == "reload": try: diff --git a/test/test_admin.py b/test/test_admin.py index 9380115d..74266d17 100644 --- a/test/test_admin.py +++ b/test/test_admin.py @@ -184,6 +184,31 @@ def testBasicInteractive(self): AdminTool.my_input = orig_input + # test EOF exit + inputs = ["help"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + # preserve directory self.install_init() + self.admin=AdminTool() + + # disable all features + self.admin.settings['history_features'] = 7 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + # 4 includes 2 commands in saved history + expected = 'roundup> exit...' + self.assertIn(expected, out) + def testGet(self): ''' Note the tests will fail if you run this under pdb. the context managers capture the pdb prompts and this screws @@ -1947,6 +1972,120 @@ def testReadline(self): self.assertIn(expected, out) + # --- test 3,4 - make sure readline gets history_length pragma. + # test CLI and interactive. + + inputs = ["pragma list", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable all config/history + self.admin.settings['history_features'] = 7 + sys.argv=['main', '-i', self.dirname, '-P', 'history_length=11'] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + self.assertEqual(self.admin.readline.get_history_length(), + 11) + + # 4 + inputs = ["pragma history_length=17", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable all config/history + self.admin.settings['history_features'] = 7 + # keep pragma in CLI. Make sure it's overridden by interactive + sys.argv=['main', '-i', self.dirname, '-P', 'history_length=11'] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + # should not be 11. + self.assertEqual(self.admin.readline.get_history_length(), + 17) + + # --- test 5 invalid single word parameter + + inputs = ["readline nosuchdirective", "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + + expected = ('roundup> Unknown readline parameter nosuchdirective') + + self.assertIn(expected, out) + + # --- test 6 set keystroke command. + # FIXME: unable to test key binding/setting actually works. + # + # No errors seem to come back from readline or + # pyreadline3 even when the keybinding makes no + # sense. Errors are only reported when reading + # from init file. Using "set answer 42" does print + # 'readline: answer: unknown variable name' when + # attached to tty/pty and interactive, but not + # inside test case. Pyreadline3 doesn't + # report errors at all. + # + # Even if I set a keybidning, I can't invoke it + # because I am not running inside a pty, so + # editing is disabled and I have no way to + # simulate keyboard keystrokes for readline to act + # upon. + + inputs = ['readline set meaning 42', "q"] + + self._monkeypatch.setattr( + 'sys.stdin', + io.StringIO("\n".join(inputs))) + + self.install_init() + self.admin=AdminTool() + + # disable loading and saving history + self.admin.settings['history_features'] = 3 + sys.argv=['main', '-i', self.dirname] + + with captured_output() as (out, err): + ret = self.admin.main() + out = out.getvalue().strip().split('\n') + + print(ret) + self.assertTrue(ret == 0) + # === cleanup if original_home: os.environ['HOME'] = original_home From 882c577fa3876bbe9345adeacdde15f33ad88eb1 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 1 Sep 2025 17:48:57 -0400 Subject: [PATCH 22/26] doc: fix typos. --- CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index fb3b06d3..68aa8d84 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -24,7 +24,7 @@ Fixed: rather than Exception since it is an HTTP exception. (John Rouillard) - cleaned up repo. Close obsolete branches and close a split head due - to an identical merge intwo different workicng copies. (John + to an identical merge in two different working copies. (John Rouillard) - in roundup-admin, using 'pragma history_length interactively now sets readline history length. Using -P history_length=10 on the From b8e186c852121e3051a68a52c03f381973a420b5 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 1 Sep 2025 20:35:54 -0400 Subject: [PATCH 23/26] fix missing formatting --- doc/admin_guide.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index c08fee98..bebf7402 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -117,7 +117,7 @@ comments: a comment and is stripped from the file before being passed to the json parser. This is a "line comment". -2. A ``//` with at least three white space characters before it is +2. A ``//`` with at least three white space characters before it is stripped from the end of the line before begin passed to the json parser. This is an "inline comment". From 27162c67396915f228b414031bed0afb045e37a9 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 1 Sep 2025 21:54:48 -0400 Subject: [PATCH 24/26] feat: allow admin to set logging format from config.ini This is prep work for adding a per thread logging variable that can be used to tie all logs for a single request together. This uses the same default logging format as before, just moves it to config.ini. Also because of configparser, the logging format has to have doubled % signs. So use: %%(asctime)s not '%(asctime)s' as configparser tries to interpolate that string and asctime is not defined in the configparser's scope. Using %%(asctime)s is not interpolated by configparser and is passed into Roundup. --- CHANGES.txt | 2 ++ doc/admin_guide.txt | 20 +++++++++++++++- roundup/configuration.py | 52 +++++++++++++++++++++++++++++++++++++--- test/test_config.py | 37 ++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 68aa8d84..4ec37e17 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -42,6 +42,8 @@ Features: - add readline command to roundup-admin to list history, control input mode etc. Also support bang (!) commands to rerun commands in history or put them in the input buffer for editing. (John Rouillard) +- add format to logging section in config.ini. Used to set default + logging format. (John Rouillard) 2025-07-13 2.5.0 diff --git a/doc/admin_guide.txt b/doc/admin_guide.txt index bebf7402..ad92a94a 100644 --- a/doc/admin_guide.txt +++ b/doc/admin_guide.txt @@ -63,6 +63,8 @@ Configuration for "BasicLogging" implementation: - tracker configuration file lets you disable other loggers (e.g. when running under a wsgi framework) with ``logging`` -> ``disable_loggers``. + - tracker configuration file can set the log format using + ``logging`` -> ``format``. See :ref:`logFormat` for more info. - ``roundup-server`` specifies the location of a log file on the command line - ``roundup-server`` enable using the standard python logger with @@ -83,10 +85,26 @@ Configuration for standard "logging" module: In both cases, if no logfile is specified then logging will simply be sent to sys.stderr with only logging of ERROR messages. +.. _logFormat: + +Defining the Log Format +----------------------- + +Starting with Roundup 2.6 you can specify the logging format. In the +``logging`` -> ``format`` setting of config.ini you can use any of the +`standard logging LogRecord attributes +`_. +However you must double any ``%`` format markers. The default value +is:: + + %%(asctime)s %%(levelname)s %%(message)s + Standard Logging Setup ---------------------- -You can specify your log configs in one of two formats: +If the settings in config.ini are not sufficient for your logging +requirements, you can specify a full logging configuration in one of +two formats: * `fileConfig format `_ diff --git a/roundup/configuration.py b/roundup/configuration.py index d170eeed..6da56200 100644 --- a/roundup/configuration.py +++ b/roundup/configuration.py @@ -273,7 +273,8 @@ def load_ini(self, config): try: if config.has_option(self.section, self.setting): self.set(config.get(self.section, self.setting)) - except configparser.InterpolationSyntaxError as e: + except (configparser.InterpolationSyntaxError, + configparser.InterpolationMissingOptionError) as e: raise ParsingOptionError( _("Error in %(filepath)s with section [%(section)s] at " "option %(option)s: %(message)s") % { @@ -575,6 +576,48 @@ def get(self): return None +class LoggingFormatOption(Option): + """Replace escaped % (as %%) with single %. + + Config file parsing allows variable interpolation using + %(keyname)s. However this is exactly the format that we need + for creating a logging format string. So we tell the user to + quote the string using %%(...). Then we turn %%( -> %( when + retrieved. + """ + + class_description = ("Allowed value: Python logging module named " + "attributes with % sign doubled.") + + def str2value(self, value): + """Check format of unquoted string looking for missing specifiers. + + This does a dirty check to see if a token is missing a + specifier. So "%(ascdate)s %(level) " would fail because of + the 's' missing after 'level)'. But "%(ascdate)s %(level)s" + would pass. + + Note that %(foo)s generates a error from the ini parser + with a less than wonderful message. + """ + unquoted_val = value.replace("%%(", "%(") + + # regexp matches all current logging record object attribute names. + scanned_result = re.sub(r'%\([A-Za-z_]+\)\S','', unquoted_val ) + if scanned_result.find('%(') != -1: + raise OptionValueError( + self, unquoted_val, + "Check that all substitution tokens have a format " + "specifier after the ). Unrecognized use of %%(...) in: " + "%s" % scanned_result) + + return str(unquoted_val) + + def _value2str(self, value): + """Replace %( with %%( to quote the format substitution param. + """ + return value.replace("%(", "%%(") + class OriginHeadersListOption(Option): """List of space seperated origin header values. @@ -1614,6 +1657,10 @@ def str2value(self, value): "Minimal severity level of messages written to log file.\n" "If above 'config' option is set, this option has no effect.\n" "Allowed values: DEBUG, INFO, WARNING, ERROR"), + (LoggingFormatOption, "format", + "%(asctime)s %(levelname)s %(message)s", + "Format of the logging messages with all '%' signs\n" + "doubled so they are not interpreted by the config file."), (BooleanOption, "disable_loggers", "no", "If set to yes, only the loggers configured in this section will\n" "be used. Yes will disable gunicorn's --access-logfile.\n"), @@ -2448,8 +2495,7 @@ def init_logging(self): hdlr = logging.FileHandler(_file) if _file else \ logging.StreamHandler(sys.stdout) - formatter = logging.Formatter( - '%(asctime)s %(levelname)s %(message)s') + formatter = logging.Formatter(self["LOGGING_FORMAT"]) hdlr.setFormatter(formatter) # no logging API to remove all existing handlers!?! for h in logger.handlers: diff --git a/test/test_config.py b/test/test_config.py index 130c9c41..b3547cdf 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -1112,6 +1112,43 @@ def testInvalidIndexerValue(self): self.assertIn("nati", string_rep) self.assertIn("'whoosh'", string_rep) + def testLoggerFormat(self): + config = configuration.CoreConfig() + + # verify config is initalized to defaults + self.assertEqual(config['LOGGING_FORMAT'], + '%(asctime)s %(levelname)s %(message)s') + + # load config + config.load(self.dirname) + self.assertEqual(config['LOGGING_FORMAT'], + '%(asctime)s %(levelname)s %(message)s') + + # break config using an incomplete format specifier (no trailing 's') + self.munge_configini(mods=[ ("format = ", "%%(asctime)s %%(levelname) %%(message)s") ], section="[logging]") + + # load config + with self.assertRaises(configuration.OptionValueError) as cm: + config.load(self.dirname) + + self.assertIn('Unrecognized use of %(...) in: %(levelname)', + cm.exception.args[2]) + + # break config by not dubling % sign to quote it from configparser + self.munge_configini(mods=[ ("format = ", "%(asctime)s %%(levelname) %%(message)s") ], section="[logging]") + + with self.assertRaises( + configuration.ParsingOptionError) as cm: + config.load(self.dirname) + + self.assertEqual(cm.exception.args[0], + "Error in _test_instance/config.ini with section " + "[logging] at option format: Bad value substitution: " + "option 'format' in section 'logging' contains an " + "interpolation key 'asctime' which is not a valid " + "option name. Raw value: '%(asctime)s %%(levelname) " + "%%(message)s'") + def testDictLoggerConfigViaJson(self): # good base test case From cf70a3778c7b12caebeb367f9e490c90ba60c597 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 8 Sep 2025 09:07:12 -0400 Subject: [PATCH 25/26] chore: dependabot upgrade setup-python to 6.0.0 --- .github/workflows/build-xapian.yml | 2 +- .github/workflows/ci-test.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-xapian.yml b/.github/workflows/build-xapian.yml index 00e6fe09..01384615 100644 --- a/.github/workflows/build-xapian.yml +++ b/.github/workflows/build-xapian.yml @@ -46,7 +46,7 @@ jobs: # Setup version of Python to use - name: Set Up Python 3.13 - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0 with: python-version: 3.13 allow-prereleases: true diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index 35a5a948..c9dac0ce 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -120,7 +120,7 @@ jobs: # Setup version of Python to use - name: Set Up Python ${{ matrix.python-version }} - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6.0.0 with: python-version: ${{ matrix.python-version }} allow-prereleases: true From b9b8478fc2e37b77790b6c099e091b344c5ebe72 Mon Sep 17 00:00:00 2001 From: John Rouillard Date: Mon, 8 Sep 2025 09:08:14 -0400 Subject: [PATCH 26/26] chore: dependabot upgrade codecov to 5.5.1 --- .github/workflows/ci-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index c9dac0ce..a5498a88 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -323,7 +323,7 @@ jobs: - name: Upload coverage to Codecov # see: https://github.com/codecov/codecov-action#usage - uses: codecov/codecov-action@fdcc8476540edceab3de004e990f80d881c6cc00 # v5.5.0 + uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1 with: verbose: true token: ${{ secrets.CODECOV_TOKEN }}