From 0c1f304943b86d10054691d4d8160f708aa555f2 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Thu, 18 May 2023 09:50:47 +0530 Subject: [PATCH 1/3] gh-103606: Improve error message from logging.config.FileConfig (GH-103628) (cherry picked from commit 152227b569c3a9b87fe0483706f704762ced6d75) --- Doc/library/logging.config.rst | 8 ++++++++ Lib/logging/config.py | 23 ++++++++++++++++------ Lib/test/test_logging.py | 36 ++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 6 deletions(-) diff --git a/Doc/library/logging.config.rst b/Doc/library/logging.config.rst index 45dc975db474e7..72a1fea88ec54c 100644 --- a/Doc/library/logging.config.rst +++ b/Doc/library/logging.config.rst @@ -87,6 +87,10 @@ in :mod:`logging` itself) and defining handlers which are declared either in provides a mechanism to present the choices and load the chosen configuration). + It will raise :exc:`FileNotFoundError` if the file + doesn't exist and :exc:`ValueError` if the file is invalid or + empty. + :param fname: A filename, or a file-like object, or an instance derived from :class:`~configparser.RawConfigParser`. If a ``RawConfigParser``-derived instance is passed, it is used as @@ -126,6 +130,10 @@ in :mod:`logging` itself) and defining handlers which are declared either in .. versionadded:: 3.10 The *encoding* parameter is added. + .. versionadded:: 3.12 + An exception will be thrown if the provided file + doesn't exist or is invalid or empty. + .. function:: listen(port=DEFAULT_LOGGING_CONFIG_PORT, verify=None) Starts up a socket server on the specified port, and listens for new diff --git a/Lib/logging/config.py b/Lib/logging/config.py index f9ef7b53a3d648..34906a5d6cd14d 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -28,6 +28,8 @@ import io import logging import logging.handlers +import os +import queue import re import struct import threading @@ -58,15 +60,24 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True, encoding=Non """ import configparser + if isinstance(fname, str): + if not os.path.exists(fname): + raise FileNotFoundError(f"{fname} doesn't exist") + elif not os.path.getsize(fname): + raise ValueError(f'{fname} is an empty file') + if isinstance(fname, configparser.RawConfigParser): cp = fname else: - cp = configparser.ConfigParser(defaults) - if hasattr(fname, 'readline'): - cp.read_file(fname) - else: - encoding = io.text_encoding(encoding) - cp.read(fname, encoding=encoding) + try: + cp = configparser.ConfigParser(defaults) + if hasattr(fname, 'readline'): + cp.read_file(fname) + else: + encoding = io.text_encoding(encoding) + cp.read(fname, encoding=encoding) + except configparser.ParsingError as e: + raise ValueError(f'{fname} is invalid: {e}') formatters = _create_formatters(cp) diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 350f4a57e2bbee..9a1da8a2cd3d57 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -1645,6 +1645,42 @@ def test_config_set_handler_names(self): self.apply_config(test_config) self.assertEqual(logging.getLogger().handlers[0].name, 'hand1') + def test_exception_if_confg_file_is_invalid(self): + test_config = """ + [loggers] + keys=root + + [handlers] + keys=hand1 + + [formatters] + keys=form1 + + [logger_root] + handlers=hand1 + + [handler_hand1] + class=StreamHandler + formatter=form1 + + [formatter_form1] + format=%(levelname)s ++ %(message)s + + prince + """ + + file = io.StringIO(textwrap.dedent(test_config)) + self.assertRaises(ValueError, logging.config.fileConfig, file) + + def test_exception_if_confg_file_is_empty(self): + fd, fn = tempfile.mkstemp(prefix='test_empty_', suffix='.ini') + os.close(fd) + self.assertRaises(ValueError, logging.config.fileConfig, fn) + os.remove(fn) + + def test_exception_if_config_file_does_not_exist(self): + self.assertRaises(FileNotFoundError, logging.config.fileConfig, 'filenotfound') + def test_defaults_do_no_interpolation(self): """bpo-33802 defaults should not get interpolated""" ini = textwrap.dedent(""" From 687317a8e224cfb173157a2f0dadbfb7956ca133 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 20 May 2023 10:23:46 -0700 Subject: [PATCH 2/3] versionchanged:: 3.11.4 --- Doc/library/logging.config.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/library/logging.config.rst b/Doc/library/logging.config.rst index 72a1fea88ec54c..2d7e204e24d4f4 100644 --- a/Doc/library/logging.config.rst +++ b/Doc/library/logging.config.rst @@ -130,7 +130,7 @@ in :mod:`logging` itself) and defining handlers which are declared either in .. versionadded:: 3.10 The *encoding* parameter is added. - .. versionadded:: 3.12 + .. versionchanged:: 3.11.4 An exception will be thrown if the provided file doesn't exist or is invalid or empty. From c8149bca8b797db149f25b5933e55a57e8d1261a Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Sun, 21 May 2023 07:37:45 +0530 Subject: [PATCH 3/3] backport #104701 --- Doc/library/logging.config.rst | 2 +- Lib/logging/config.py | 4 ++-- Lib/test/test_logging.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Doc/library/logging.config.rst b/Doc/library/logging.config.rst index 2d7e204e24d4f4..619e0406066809 100644 --- a/Doc/library/logging.config.rst +++ b/Doc/library/logging.config.rst @@ -88,7 +88,7 @@ in :mod:`logging` itself) and defining handlers which are declared either in configuration). It will raise :exc:`FileNotFoundError` if the file - doesn't exist and :exc:`ValueError` if the file is invalid or + doesn't exist and :exc:`RuntimeError` if the file is invalid or empty. :param fname: A filename, or a file-like object, or an instance derived diff --git a/Lib/logging/config.py b/Lib/logging/config.py index 34906a5d6cd14d..7e78a64a2f08a6 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -64,7 +64,7 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True, encoding=Non if not os.path.exists(fname): raise FileNotFoundError(f"{fname} doesn't exist") elif not os.path.getsize(fname): - raise ValueError(f'{fname} is an empty file') + raise RuntimeError(f'{fname} is an empty file') if isinstance(fname, configparser.RawConfigParser): cp = fname @@ -77,7 +77,7 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True, encoding=Non encoding = io.text_encoding(encoding) cp.read(fname, encoding=encoding) except configparser.ParsingError as e: - raise ValueError(f'{fname} is invalid: {e}') + raise RuntimeError(f'{fname} is invalid: {e}') formatters = _create_formatters(cp) diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 9a1da8a2cd3d57..76bd8724e29b41 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -1670,12 +1670,12 @@ def test_exception_if_confg_file_is_invalid(self): """ file = io.StringIO(textwrap.dedent(test_config)) - self.assertRaises(ValueError, logging.config.fileConfig, file) + self.assertRaises(RuntimeError, logging.config.fileConfig, file) def test_exception_if_confg_file_is_empty(self): fd, fn = tempfile.mkstemp(prefix='test_empty_', suffix='.ini') os.close(fd) - self.assertRaises(ValueError, logging.config.fileConfig, fn) + self.assertRaises(RuntimeError, logging.config.fileConfig, fn) os.remove(fn) def test_exception_if_config_file_does_not_exist(self):