From 78182b2eac6e11dd9ed1bda413996f983fe0484c Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Wed, 19 Apr 2023 17:31:09 +0530 Subject: [PATCH 01/11] gh-103606: Improve error message from logging.config.FileConfig --- Lib/logging/config.py | 5 +++-- Lib/test/test_logging.py | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Lib/logging/config.py b/Lib/logging/config.py index 16c54a6a4f7a2f..763cea080e320e 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -68,8 +68,9 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True, encoding=Non cp.read_file(fname) else: encoding = io.text_encoding(encoding) - cp.read(fname, encoding=encoding) - + files = cp.read(fname, encoding=encoding) + if not files: + raise ValueError(f"{fname} doesn't exist") formatters = _create_formatters(cp) # critical section diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 9176d8eeb56d01..322fe1bc43c389 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -1756,6 +1756,10 @@ def test_config_set_handler_names(self): self.apply_config(test_config) self.assertEqual(logging.getLogger().handlers[0].name, 'hand1') + def test_exception_if_file_not_present(self): + """Test if not present file raises ValueError""" + self.assertRaises(ValueError, logging.config.fileConfig, 'randfile') + def test_defaults_do_no_interpolation(self): """bpo-33802 defaults should not get interpolated""" ini = textwrap.dedent(""" From 2c08f30e0f4a3233020fb9d7ebb7c8fbcae181a2 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Wed, 19 Apr 2023 17:31:09 +0530 Subject: [PATCH 02/11] gh-103606: Improve error message from logging.config.FileConfig --- Lib/logging/config.py | 24 +++++++++++++++++------- Lib/test/test_logging.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/Lib/logging/config.py b/Lib/logging/config.py index 16c54a6a4f7a2f..8e164d200a90da 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -29,6 +29,7 @@ import io import logging import logging.handlers +import os import queue import re import struct @@ -59,17 +60,26 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True, encoding=Non configuration). """ import configparser + + if isinstance(fname, str): + if not os.path.exists(fname): + raise FileNotFoundError(f"{fname} doesn't exit") + elif not os.path.getsize(fname): + raise ValueError(f"{fname} configuration file is empty") 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, errored with: {e}") + formatters = _create_formatters(cp) # critical section diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 9176d8eeb56d01..e25daa5bbe5b07 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -1756,6 +1756,40 @@ 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): + _, filename = tempfile.mkstemp() + self.assertRaises(ValueError, logging.config.fileConfig, filename) + + def test_exception_if_confg_file_does_not_exists(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 c75b986063e85ad91f44f4e9a6ad202704c9cfae Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Mon, 15 May 2023 13:52:08 +0530 Subject: [PATCH 03/11] Update Lib/logging/config.py Co-authored-by: Vinay Sajip --- Lib/logging/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/logging/config.py b/Lib/logging/config.py index 2e2d2acdd5153b..031c6cf6ed31b7 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -63,7 +63,7 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True, encoding=Non if isinstance(fname, str): if not os.path.exists(fname): - raise FileNotFoundError(f"{fname} doesn't exit") + raise FileNotFoundError(f"{fname} doesn't exist") elif not os.path.getsize(fname): raise ValueError(f"{fname} configuration file is empty") From deb3c3e637925d045c94f7c92e5b7aab831ed1ad Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Mon, 15 May 2023 13:52:20 +0530 Subject: [PATCH 04/11] Update Lib/logging/config.py Co-authored-by: Vinay Sajip --- Lib/logging/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/logging/config.py b/Lib/logging/config.py index 031c6cf6ed31b7..c833222be8aae8 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -65,7 +65,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} configuration file is empty") + raise ValueError(f'{fname} is an empty file') if isinstance(fname, configparser.RawConfigParser): cp = fname From 0f299c003be3e4a711a593f1a44b3d04968fdb35 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Mon, 15 May 2023 13:52:29 +0530 Subject: [PATCH 05/11] Update Lib/logging/config.py Co-authored-by: Vinay Sajip --- Lib/logging/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/logging/config.py b/Lib/logging/config.py index c833222be8aae8..652f21ecb459f1 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -78,7 +78,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, errored with: {e}") + raise ValueError(f'{fname} is invalid: {e}') formatters = _create_formatters(cp) From c927ac32d207b71d93524b57a8d88e047d10b5bb Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Mon, 15 May 2023 13:52:38 +0530 Subject: [PATCH 06/11] Update Lib/test/test_logging.py Co-authored-by: Vinay Sajip --- Lib/test/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index da97df5fd0dbc2..b348a5700356c8 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -1789,7 +1789,7 @@ def test_exception_if_confg_file_is_empty(self): self.assertRaises(ValueError, logging.config.fileConfig, fn) os.unlink(fn) - def test_exception_if_confg_file_does_not_exists(self): + def test_exception_if_config_file_does_not_exist(self): self.assertRaises(FileNotFoundError, logging.config.fileConfig, 'filenotfound') def test_defaults_do_no_interpolation(self): From a9cc7c0fbf99ea6516b936feb1dfbad8ffab876f Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Mon, 15 May 2023 13:52:47 +0530 Subject: [PATCH 07/11] Update Lib/test/test_logging.py Co-authored-by: Vinay Sajip --- Lib/test/test_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index b348a5700356c8..ba836a7d9ea312 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -1787,7 +1787,7 @@ 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.unlink(fn) + os.remove(fn) def test_exception_if_config_file_does_not_exist(self): self.assertRaises(FileNotFoundError, logging.config.fileConfig, 'filenotfound') From 96a9260be9ec02e3455b4a21e2809b6e9c9a1aa8 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Mon, 15 May 2023 14:25:38 +0530 Subject: [PATCH 08/11] add logging improvement in whatsnew section --- Doc/whatsnew/3.12.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index eb13d4bf031c95..ceaa402439dc01 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -420,6 +420,13 @@ itertools tuples where the last batch may be shorter than the rest. (Contributed by Raymond Hettinger in :gh:`98363`.) +logging +------- + +* :func:`logging.config.fileConfig` now raise :exc:`FileNotFoundError` + if file doesn't exist and :exc:`ValueError` if file is invalid or + empty. (Contributed by Prince Roshan in :gh:`103606`.) + math ---- From 1c0ce20981cdf8d29bc23b1406be338b5f5ce941 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Mon, 15 May 2023 20:26:47 +0530 Subject: [PATCH 09/11] Add doc --- Doc/library/logging.config.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Doc/library/logging.config.rst b/Doc/library/logging.config.rst index 250246b5cd9adc..fe14a1b3976e9e 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 file + doesn't exist and :exc:`ValueError` if 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 + The exception will be thrown if the file provided + 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 From 340b860f4093aef177fbdd4c708a54cd433aabc7 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Tue, 16 May 2023 17:50:54 +0530 Subject: [PATCH 10/11] fix doc --- Doc/library/logging.config.rst | 6 +++--- Doc/whatsnew/3.12.rst | 7 ------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/Doc/library/logging.config.rst b/Doc/library/logging.config.rst index fe14a1b3976e9e..80fc5b8fded9cf 100644 --- a/Doc/library/logging.config.rst +++ b/Doc/library/logging.config.rst @@ -88,8 +88,8 @@ in :mod:`logging` itself) and defining handlers which are declared either in configuration). It will raise :exc:`FileNotFoundError` if file - doesn't exist and :exc:`ValueError` if file is invalid or - empty + 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 @@ -131,7 +131,7 @@ in :mod:`logging` itself) and defining handlers which are declared either in The *encoding* parameter is added. .. versionadded:: 3.12 - The exception will be thrown if the file provided + 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) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index ceaa402439dc01..eb13d4bf031c95 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -420,13 +420,6 @@ itertools tuples where the last batch may be shorter than the rest. (Contributed by Raymond Hettinger in :gh:`98363`.) -logging -------- - -* :func:`logging.config.fileConfig` now raise :exc:`FileNotFoundError` - if file doesn't exist and :exc:`ValueError` if file is invalid or - empty. (Contributed by Prince Roshan in :gh:`103606`.) - math ---- From 5fb63b0e34c791ae2a12fe58980857c1a77daca4 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Tue, 16 May 2023 17:52:11 +0530 Subject: [PATCH 11/11] Update Doc/library/logging.config.rst Co-authored-by: Vinay Sajip --- 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 80fc5b8fded9cf..2fb05c67005735 100644 --- a/Doc/library/logging.config.rst +++ b/Doc/library/logging.config.rst @@ -87,7 +87,7 @@ 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 file + It will raise :exc:`FileNotFoundError` if the file doesn't exist and :exc:`ValueError` if the file is invalid or empty.