8000 [3.12] gh-115809: Improve TimedRotatingFileHandler.getFilesToDelete()… · python/cpython@3f830ad · GitHub
[go: up one dir, main page]
Skip to content

Commit 3f830ad

Browse files
miss-islingtonserhiy-storchaka
miss-islington
and
serhiy-storchaka
authored
[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 87faec2) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent 5b73ed4 commit 3f830ad

File tree

3 files changed

+56
-36
lines changed

3 files changed

+56
-36
lines changed

‎Lib/logging/handlers.py

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -232,19 +232,19 @@ def __init__(self, filename, when='h', interval=1, backupCount=0,
232232
if self.when == 'S':
233233
self.interval = 1 # one second
234234
self.suffix = "%Y-%m-%d_%H-%M-%S"
235-
self.extMatch = r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}(\.\w+)?$"
235+
extMatch = r"(?<!\d)\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}(?!\d)"
236236
elif self.when == 'M':
237237
self.interval = 60 # one minute
238238
self.suffix = "%Y-%m-%d_%H-%M"
239-
self.extMatch = r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}(\.\w+)?$"
239+
extMatch = r"(?<!\d)\d{4}-\d{2}-\d{2}_\d{2}-\d{2}(?!\d)"
240240
elif self.when == 'H':
241241
self.interval = 60 * 60 # one hour
242242
self.suffix = "%Y-%m-%d_%H"
243-
self.extMatch = r"^\d{4}-\d{2}-\d{2}_\d{2}(\.\w+)?$"
243+
extMatch = r"(?<!\d)\d{4}-\d{2}-\d{2}_\d{2}(?!\d)"
244244
elif self.when == 'D' or self.when == 'MIDNIGHT':
245245
self.interval = 60 * 60 * 24 # one day
246246
self.suffix = "%Y-%m-%d"
247-
self.extMatch = r"^\d{4}-\d{2}-\d{2}(\.\w+)?$"
247+
extMatch = r"(?<!\d)\d{4}-\d{2}-\d{2}(?!\d)"
248248
elif self.when.startswith('W'):
249249
self.interval = 60 * 60 * 24 * 7 # one week
250250
if len(self.when) != 2:
@@ -253,11 +253,17 @@ def __init__(self, filename, when='h', interval=1, backupCount=0,
253253
raise ValueError("Invalid day specified for weekly rollover: %s" % self.when)
254254
self.dayOfWeek = int(self.when[1])
255255
self.suffix = "%Y-%m-%d"
256-
self.extMatch = r"^\d{4}-\d{2}-\d{2}(\.\w+)?$"
256+
extMatch = r"(?<!\d)\d{4}-\d{2}-\d{2}(?!\d)"
257257
else:
258258
raise ValueError("Invalid rollover interval specified: %s" % self.when)
259259

260-
self.extMatch = re.compile(self.extMatch, re.ASCII)
260+
# extMatch is a pattern for matching a datetime suffix in a file name.
261+
# After custom naming, it is no longer guaranteed to be separated by
262+
# periods from other parts of the filename. The lookup statements
263+
# (?<!\d) and (?!\d) ensure that the datetime suffix (which itself
264+
# starts and ends with digits) is not preceded or followed by digits.
265+
# This reduces the number of false matches and improves performance.
266+
self.extMatch = re.compile(extMatch, re.ASCII)
261267
self.interval = self.interval * interval # multiply by units requested
262268
# The following line added because the filename passed in could be a
263269
# path object (see Issue #27493), but self.baseFilename will be a string
@@ -376,31 +382,22 @@ def getFilesToDelete(self):
376382
for fileName in fileNames:
377383
if fileName[:plen] == prefix:
378384
suffix = fileName[plen:]
379-
if self.extMatch.match(suffix):
385+
if self.extMatch.fullmatch(suffix):
380386
result.append(os.path.join(dirName, fileName))
381387
else:
382-
# See bpo-44753: Don't use the extension when computing the prefix.
383-
n, e = os.path.splitext(baseName)
384-
prefix = n + '.'
385-
plen = len(prefix)
386388
for fileName in fileNames:
387-
# Our files could be just about anything after custom naming, but
388-
# likely candidates are of the form
389-
# foo.log.DATETIME_SUFFIX or foo.DATETIME_SUFFIX.log
390-
if (not fileName.startswith(baseName) and fileName.endswith(e) and
391-
len(fileName) > (plen + 1) and not fileName[plen+1].isdigit()):
392-
continue
389+
# Our files could be just about anything after custom naming,
390+
# but they should contain the datetime suffix.
391+
# Try to find the datetime suffix in the file name and verify
392+
# that the file name can be generated by this handler.
393+
m = self.extMatch.search(fileName)
394+
while m:
395+
dfn = self.namer(self.baseFilename + "." + m[0])
396+
if os.path.basename(dfn) == fileName:
397+
result.append(os.path.join(dirName, fileName))
398+
break
399+
m = self.extMatch.search(fileName, m.start() + 1)
393400

394-
if fileName[:plen] == prefix:
395-
suffix = fileName[plen:]
396-
# See bpo-45628: The date/time suffix could be anywhere in the
397-
# filename
398-
399-
parts = suffix.split('.')
400-
for part in parts:
401-
if self.extMatch.match(part):
402-
result.append(os.path.join(dirName, fileName))
403-
break
404401
if len(result) < self.backupCount:
405402
result = []
406403
else:

‎Lib/test/test_logging.py

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6205,7 +6205,7 @@ def test_compute_files_to_delete(self):
62056205
for i in range(10):
62066206
times.append(dt.strftime('%Y-%m-%d_%H-%M-%S'))
62076207
dt += datetime.timedelta(seconds=5)
6208-
prefixes = ('a.b', 'a.b.c', 'd.e', 'd.e.f')
6208+
prefixes = ('a.b', 'a.b.c', 'd.e', 'd.e.f', 'g')
62096209
files = []
62106210
rotators = []
62116211
for prefix in prefixes:
@@ -6218,10 +6218,22 @@ def test_compute_files_to_delete(self):
62186218
if prefix.startswith('a.b'):
62196219
for t in times:
62206220
files.append('%s.log.%s' % (prefix, t))
6221-
else:
6222-
rotator.namer = lambda name: name.replace('.log', '') + '.log'
6221+
elif prefix.startswith('d.e'):
6222+
def namer(filename):
6223+
dirname, basename = os.path.split(filename)
6224+
basename = basename.replace('.log', '') + '.log'
6225+
return os.path.join(dirname, basename)
6226+
rotator.namer = namer
62236227
for t in times:
62246228
files.append('%s.%s.log' % (prefix, t))
6229+
elif prefix == 'g':
6230+
def namer(filename):
6231+
dirname, basename = os.path.split(filename)
6232+
basename = 'g' + basename[6:] + '.oldlog'
6233+
return os.path.join(dirname, basename)
6234+
rotator.namer = namer
6235+
for t in times:
6236+
files.append('g%s.oldlog' % t)
62256237
# Create empty files
62266238
for fn in files:
62276239
p = os.path.join(wd, fn)
@@ -6231,18 +6243,23 @@ def test_compute_files_to_delete(self):
62316243
for i, prefix in enumerate(prefixes):
62326244
rotator = rotators[i]
62336245
candidates = rotator.getFilesToDelete()
6234-
self.assertEqual(len(candidates), 3)
6246+
self.assertEqual(len(candidates), 3, candidates)
62356247
if prefix.startswith('a.b'):
62366248
p = '%s.log.' % prefix
62376249
for c in candidates:
62386250
d, fn = os.path.split(c)
62396251
self.assertTrue(fn.startswith(p))
6240-
else:
6252+
elif prefix.startswith('d.e'):
62416253
for c in candidates:
62426254
d, fn = os.path.split(c)
6243-
self.assertTrue(fn.endswith('.log'))
6255+
self.assertTrue(fn.endswith('.log'), fn)
62446256
self.assertTrue(fn.startswith(prefix + '.') and
62456257
fn[len(prefix) + 2].isdigit())
6258+
elif prefix == 'g':
6259+
for c in candidates:
6260+
d, fn = os.path.split(c)
6261+
self.assertTrue(fn.endswith('.oldlog'))
6262+
self.assertTrue(fn.startswith('g') and fn[1].isdigit())
62466263

62476264
def test_compute_files_to_delete_same_filename_different_extensions(self):
62486265
# See GH-93205 for background
@@ -6266,6 +6283,8 @@ def test_compute_files_to_delete_same_filename_different_extensions(self):
62666283
rotators.append(rotator)
62676284
for t in times:
62686285
files.append('%s.%s' % (prefix, t))
6286+
for t in times:
6287+
files.append('a.log.%s.c' % t)
62696288
# Create empty files
62706289
for f in files:
62716290
(wd / f).touch()
@@ -6274,11 +6293,11 @@ def test_compute_files_to_delete_same_filename_different_extensions(self):
62746293
backupCount = i+1
62756294
rotator = rotators[i]
62766295
candidates = rotator.getFilesToDelete()
6277-
self.assertEqual(len(candidates), n_files - backupCount)
6278-
matcher = re.compile(r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}(\.\w+)?$")
6296+
self.assertEqual(len(candidates), n_files - backupCount, candidates)
6297+
matcher = re.compile(r"^\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}\Z")
62796298
for c in candidates:
62806299
d, fn = os.path.split(c)
6281-
self.assertTrue(fn.startswith(prefix))
6300+
self.assertTrue(fn.startswith(prefix+'.'))
62826301
suffix = fn[(len(prefix)+1):]
62836302
self.assertRegex(suffix, matcher)
62846303

‎Misc/NEWS.d/next/Library/2024-02-22-11-29-27.gh-issue-115809.9H1DhB.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Improve algorithm for computing which rolled-over log files to delete in
2+
:class:`logging.TimedRotatingFileHandler`. It is now reliable for handlers
3+
without ``namer`` and with arbitrary deterministic ``namer`` that leaves the
4+
datetime part in the file name unmodified.

0 commit comments

Comments
 (0)
0