diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index 1a1337729e0c4c..d7c404e9ff6c0c 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -232,19 +232,19 @@ def __init__(self, filename, when='h', interval=1, backupCount=0, 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 e3e9f08a089aae..574f9517da4128 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -5834,7 +5834,7 @@ def test_compute_files_to_delete(self): 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: @@ -5847,10 +5847,22 @@ def test_compute_files_to_delete(self): 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) @@ -5860,18 +5872,23 @@ def test_compute_files_to_delete(self): 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 @@ -5895,6 +5912,8 @@ def test_compute_files_to_delete_same_filename_different_extensions(self): 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() @@ -5903,11 +5922,11 @@ def test_compute_files_to_delete_same_filename_different_extensions(self): 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 00000000000000..2a47efbae5c84d --- /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.