Skip to content

gh-90535: Warn of deprecation of intervals other than 1 with when='MIDNIGHT' #98099

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

gh-90535: Warn of deprecation of intervals other than 1 with when='MIDNIGHT' #98099

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40e6f6d
gh-90535: Warn of deprecation of intervals other than 1 with when='MI…
vsajip Oct 8, 2022
c7748bf
Merge branch 'main' into fix-90535
erlend-aasland Jan 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Lib/logging/handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import queue
import threading
import copy
import warnings
Copy link
Member

Choose a reason for hiding this comment

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

It is better to defer the warnings module import until it is needed (which never happen in normal case).


#
# Some constants...
Expand Down Expand Up @@ -258,6 +259,11 @@ def __init__(self, filename, when='h', interval=1, backupCount=0,
raise ValueError("Invalid rollover interval specified: %s" % self.when)

self.extMatch = re.compile(self.extMatch, re.ASCII)
if self.when == 'MIDNIGHT' and interval != 1:
msg = (f'An interval of {interval} is incompatible with '
"when='MIDNIGHT'. Use when='D' (for days) "
'if you want to use an interval other than 1.')
warnings.warn(msg, category=DeprecationWarning, stacklevel=2)
self.interval = self.interval * interval # multiply by units requested
# The following line added because the filename passed in could be a
# path object (see Issue #27493), but self.baseFilename will be a string
Expand Down
10 changes: 10 additions & 0 deletions Lib/test/test_logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5755,6 +5755,16 @@ def test_compute_rollover_daily_attime(self):
finally:
rh.close()

def test_midnight_interval(self):
with warnings.catch_warnings(record=True) as w:
rh = logging.handlers.TimedRotatingFileHandler(
self.fn, encoding="utf-8", when='MIDNIGHT', interval=2, backupCount=0,
utc=True)
rh.close()
if w:
Copy link
Member

Choose a reason for hiding this comment

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

Do you want the test to pass if no warning is raised? If not then in the above I would force deprecations on as exceptions via warnings.simplefilter("error").

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Thanks for the review, Brett.

Copy link
Member

Choose a reason for hiding this comment

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

Why not use assertWarns()?

self.assertIs(w[0].category, DeprecationWarning)
self.assertIn('An interval of 2 is incompatible ', str(w[0].message))

#@unittest.skipIf(True, 'Temporarily skipped while failures investigated.')
def test_compute_rollover_weekly_attime(self):
currentTime = int(time.time())
Expand Down