Skip to content

gh-93205: When rotating logs with no namer specified, match whole extension #93224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

ilCatania
Copy link
Contributor

@ilCatania ilCatania commented May 25, 2022

Fixes GH-93205: When setting up multiple logging handlers of type TimedRotatingFileHandler that log to the same directory and only differ by the file extension, rollover does not work properly for either of them.

There have been a few changes in TimeRotatingHandler (GH-90221, GH-89791) due to different file format expectations when a namer is specified versus not specified. This PR fixes the issue in GH-93205 while trying to simplify the code.

@ilCatania ilCatania requested a review from vsajip as a code owner May 25, 2022 17:35
@ghost
Copy link

ghost commented May 25, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@serhiy-storchaka serhiy-storchaka changed the title gh-93205 when rotating logs with no namer specified, match whole extension gh-93205: When rotating logs with no namer specified, match whole extension Feb 21, 2024
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I agree that it is better to separate simpler case with no namer, where the matches can be found unambiguous and more efficiently from more complex case with helper.

It is still not completely unambiguous and can conflict with other TimedRotatingFileHandler with a namer that simply adds an extension, but I'm going to fix this in other issue.

@serhiy-storchaka serhiy-storchaka merged commit 113687a into python:main Feb 21, 2024
@miss-islington-app
Copy link

Thanks @ilCatania for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 21, 2024
…le extension (pythonGH-93224)

(cherry picked from commit 113687a)

Co-authored-by: Gabriele Catania <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 21, 2024

GH-115784 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Feb 21, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 21, 2024
…le extension (pythonGH-93224)

(cherry picked from commit 113687a)

Co-authored-by: Gabriele Catania <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 21, 2024

GH-115785 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Feb 21, 2024
serhiy-storchaka pushed a commit that referenced this pull request Feb 21, 2024
…ole extension (GH-93224) (GH-115784)

(cherry picked from commit 113687a)

Co-authored-by: Gabriele Catania <[email protected]>
serhiy-storchaka pushed a commit that referenced this pull request Feb 21, 2024
…ole extension (GH-93224) (GH-115785)

(cherry picked from commit 113687a)

Co-authored-by: Gabriele Catania <[email protected]>
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
LukasWoodtli pushed a commit to LukasWoodtli/cpython that referenced this pull request Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple TimedRotatingFileHandler with similar names but different backup counts do not work
4 participants