Skip to content

Commit

Permalink
gh-93205: When rotating logs with no namer specified, match whole ext…
Browse files Browse the repository at this point in the history
…ension (GH-93224)
  • Loading branch information
ilCatania authored Feb 21, 2024
1 parent 347acde commit 113687a
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 19 deletions.
43 changes: 24 additions & 19 deletions Lib/logging/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,32 +369,37 @@ def getFilesToDelete(self):
dirName, baseName = os.path.split(self.baseFilename)
fileNames = os.listdir(dirName)
result = []
# See bpo-44753: Don't use the extension when computing the prefix.
n, e = os.path.splitext(baseName)
prefix = n + '.'
plen = len(prefix)
for fileName in fileNames:
if self.namer is None:
# Our files will always start with baseName
if not fileName.startswith(baseName):
continue
else:
if self.namer is None:
prefix = baseName + '.'
plen = len(prefix)
for fileName in fileNames:
if fileName[:plen] == prefix:
suffix = fileName[plen:]
if self.extMatch.match(suffix):
result.append(os.path.join(dirName, fileName))
else:
# See bpo-44753: Don't use the extension when computing the prefix.
n, e = os.path.splitext(baseName)
prefix = n + '.'
plen = len(prefix)
for fileName in fileNames:
# Our files could be just about anything after custom naming, but
# likely candidates are of the form
# foo.log.DATETIME_SUFFIX or foo.DATETIME_SUFFIX.log
if (not fileName.startswith(baseName) and fileName.endswith(e) and
len(fileName) > (plen + 1) and not fileName[plen+1].isdigit()):
continue

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 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:
Expand Down
37 changes: 37 additions & 0 deletions Lib/test/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -6193,6 +6193,43 @@ def test_compute_files_to_delete(self):
self.assertTrue(fn.startswith(prefix + '.') and
fn[len(prefix) + 2].isdigit())

def test_compute_files_to_delete_same_filename_different_extensions(self):
# See GH-93205 for background
wd = pathlib.Path(tempfile.mkdtemp(prefix='test_logging_'))
self.addCleanup(shutil.rmtree, wd)
times = []
dt = datetime.datetime.now()
n_files = 10
for _ in range(n_files):
times.append(dt.strftime('%Y-%m-%d_%H-%M-%S'))
dt += datetime.timedelta(seconds=5)
prefixes = ('a.log', 'a.log.b')
files = []
rotators = []
for i, prefix in enumerate(prefixes):
backupCount = i+1
rotator = logging.handlers.TimedRotatingFileHandler(wd / prefix, when='s',
interval=5,
backupCount=backupCount,
delay=True)
rotators.append(rotator)
for t in times:
files.append('%s.%s' % (prefix, t))
# Create empty files
for f in files:
(wd / f).touch()
# Now the checks that only the correct files are offered up for deletion
for i, prefix in enumerate(prefixes):
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+)?$")
for c in candidates:
d, fn = os.path.split(c)
self.assertTrue(fn.startswith(prefix))
suffix = fn[(len(prefix)+1):]
self.assertRegex(suffix, matcher)

def secs(**kw):
return datetime.timedelta(**kw) // datetime.timedelta(seconds=1)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a bug in :class:`logging.handlers.TimedRotatingFileHandler` where multiple rotating handler instances pointing to files with the same name but different extensions would conflict and not delete the correct files.

0 comments on commit 113687a

Please sign in to comment.