From 3f830adf58745723dba5b7fcf7cb39e4c0d0224b Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Sun, 3 Mar 2024 08:59:10 +0100 Subject: [3.12] gh-115809: Improve TimedRotatingFileHandler.getFilesToDelete() (GH-115812) (GH-116261) Improve algorithm for computing which rolled-over log files to delete in logging.TimedRotatingFileHandler. It is now reliable for handlers without namer and with arbitrary deterministic namer that leaves the datetime part in the file name unmodified. (cherry picked from commit 87faec28c78f6fa8eaaebbd1ababf687c7508e71) Co-authored-by: Serhiy Storchaka --- Lib/logging/handlers.py | 51 ++++++++++------------ Lib/test/test_logging.py | 37 ++++++++++++---- .../2024-02-22-11-29-27.gh-issue-115809.9H1DhB.rst | 4 ++ 3 files changed, 56 insertions(+), 36 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-02-22-11-29-27.gh-issue-115809.9H1DhB.rst diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index 6dbee7b..af9b345 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -232,19 +232,19 @@ class TimedRotatingFileHandler(BaseRotatingHandler): if self.when == 'S': self.interval = 1 # one second self.suffix = "%Y-%m-%d_%H-%M-%S" - self.extMatch = r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}(\.\w+)?$" + extMatch = r"(? (plen + 1) and not fileName[plen+1].isdigit()): - continue + # Our files could be just about anything after custom naming, + # but they should contain the datetime suffix. + # Try to find the datetime suffix in the file name and verify + # that the file name can be generated by this handler. + m = self.extMatch.search(fileName) + while m: + dfn = self.namer(self.baseFilename + "." + m[0]) + if os.path.basename(dfn) == fileName: + result.append(os.path.join(dirName, fileName)) + break + m = self.extMatch.search(fileName, m.start() + 1) - if fileName[:plen] == prefix: - suffix = fileName[plen:] - # See bpo-45628: The date/time suffix could be anywhere in the - # filename - - parts = suffix.split('.') - for part in parts: - if self.extMatch.match(part): - result.append(os.path.join(dirName, fileName)) - break if len(result) < self.backupCount: result = [] else: diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 857f656..224447d 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -6205,7 +6205,7 @@ class TimedRotatingFileHandlerTest(BaseFileTest): for i in range(10): times.append(dt.strftime('%Y-%m-%d_%H-%M-%S')) dt += datetime.timedelta(seconds=5) - prefixes = ('a.b', 'a.b.c', 'd.e', 'd.e.f') + prefixes = ('a.b', 'a.b.c', 'd.e', 'd.e.f', 'g') files = [] rotators = [] for prefix in prefixes: @@ -6218,10 +6218,22 @@ class TimedRotatingFileHandlerTest(BaseFileTest): if prefix.startswith('a.b'): for t in times: files.append('%s.log.%s' % (prefix, t)) - else: - rotator.namer = lambda name: name.replace('.log', '') + '.log' + elif prefix.startswith('d.e'): + def namer(filename): + dirname, basename = os.path.split(filename) + basename = basename.replace('.log', '') + '.log' + return os.path.join(dirname, basename) + rotator.namer = namer for t in times: files.append('%s.%s.log' % (prefix, t)) + elif prefix == 'g': + def namer(filename): + dirname, basename = os.path.split(filename) + basename = 'g' + basename[6:] + '.oldlog' + return os.path.join(dirname, basename) + rotator.namer = namer + for t in times: + files.append('g%s.oldlog' % t) # Create empty files for fn in files: p = os.path.join(wd, fn) @@ -6231,18 +6243,23 @@ class TimedRotatingFileHandlerTest(BaseFileTest): for i, prefix in enumerate(prefixes): rotator = rotators[i] candidates = rotator.getFilesToDelete() - self.assertEqual(len(candidates), 3) + self.assertEqual(len(candidates), 3, candidates) if prefix.startswith('a.b'): p = '%s.log.' % prefix for c in candidates: d, fn = os.path.split(c) self.assertTrue(fn.startswith(p)) - else: + elif prefix.startswith('d.e'): for c in candidates: d, fn = os.path.split(c) - self.assertTrue(fn.endswith('.log')) + self.assertTrue(fn.endswith('.log'), fn) self.assertTrue(fn.startswith(prefix + '.') and fn[len(prefix) + 2].isdigit()) + elif prefix == 'g': + for c in candidates: + d, fn = os.path.split(c) + self.assertTrue(fn.endswith('.oldlog')) + self.assertTrue(fn.startswith('g') and fn[1].isdigit()) def test_compute_files_to_delete_same_filename_different_extensions(self): # See GH-93205 for background @@ -6266,6 +6283,8 @@ class TimedRotatingFileHandlerTest(BaseFileTest): rotators.append(rotator) for t in times: files.append('%s.%s' % (prefix, t)) + for t in times: + files.append('a.log.%s.c' % t) # Create empty files for f in files: (wd / f).touch() @@ -6274,11 +6293,11 @@ class TimedRotatingFileHandlerTest(BaseFileTest): backupCount = i+1 rotator = rotators[i] candidates = rotator.getFilesToDelete() - self.assertEqual(len(candidates), n_files - backupCount) - matcher = re.compile(r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}(\.\w+)?$") + self.assertEqual(len(candidates), n_files - backupCount, candidates) + matcher = re.compile(r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}\Z") for c in candidates: d, fn = os.path.split(c) - self.assertTrue(fn.startswith(prefix)) + self.assertTrue(fn.startswith(prefix+'.')) suffix = fn[(len(prefix)+1):] self.assertRegex(suffix, matcher) diff --git a/Misc/NEWS.d/next/Library/2024-02-22-11-29-27.gh-issue-115809.9H1DhB.rst b/Misc/NEWS.d/next/Library/2024-02-22-11-29-27.gh-issue-115809.9H1DhB.rst new file mode 100644 index 0000000..2a47efb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-02-22-11-29-27.gh-issue-115809.9H1DhB.rst @@ -0,0 +1,4 @@ +Improve algorithm for computing which rolled-over log files to delete in +:class:`logging.TimedRotatingFileHandler`. It is now reliable for handlers +without ``namer`` and with arbitrary deterministic ``namer`` that leaves the +datetime part in the file name unmodified. -- cgit v0.12