8000 gh-124653: Relax (again) detection of queue API for logging handlers … · python/cpython@7ffe94f · GitHub
[go: up one dir, main page]
Skip to content

Commit 7ffe94f

Browse files
picnixzpicnixz
authored
gh-124653: Relax (again) detection of queue API for logging handlers (GH-124897)
1 parent 0377547 commit 7ffe94f

File tree

4 files changed

+79
-56
lines changed

4 files changed

+79
-56
lines changed

‎Doc/library/logging.config.rst

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -753,16 +753,17 @@ The ``queue`` and ``listener`` keys are optional.
753753

754754
If the ``queue`` key is present, the corresponding value can be one of the following:
755755

756-
* An object implementing the :class:`queue.Queue` public API. For instance,
757-
this may be an actual instance of :class:`queue.Queue` or a subclass thereof,
758-
or a proxy obtained by :meth:`multiprocessing.managers.SyncManager.Queue`.
756+
* An object implementing the :meth:`Queue.put_nowait <queue.Queue.put_nowait>`
757+
and :meth:`Queue.get <queue.Queue.get>` public API. For instance, this may be
758+
an actual instance of :class:`queue.Queue` or a subclass thereof, or a proxy
759+
obtained by :meth:`multiprocessing.managers.SyncManager.Queue`.
759760

760761
This is of course only possible if you are constructing or modifying
761762
the configuration dictionary in code.
762763

763764
* A string that resolves to a callable which, when called with no arguments, returns
764-
the :class:`queue.Queue` instance to use. That callable could be a
765-
:class:`queue.Queue` subclass or a function which returns a suitable queue instance,
765+
the queue instance to use. That callable could be a :class:`queue.Queue` subclass
766+
or a function which returns a suitable queue instance,
766767
such as ``my.module.queue_factory()``.
767768

768769
* A dict with a ``'()'`` key which is constructed in the usual way as discussed in

‎Lib/logging/config.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ def as_tuple(self, value):
499499

500500
def _is_queue_like_object(obj):
501501
"""Check that *obj* implements the Queue API."""
502-
if isinstance(obj, queue.Queue):
502+
if isinstance(obj, (queue.Queue, queue.SimpleQueue)):
503503
return True
504504
# defer importing multiprocessing as much as possible
505505
from multiprocessing.queues import Queue as MPQueue
@@ -516,13 +516,13 @@ def _is_queue_like_object(obj):
516516
# Ideally, we would have wanted to simply use strict type checking
517517
# instead of a protocol-based type checking since the latter does
518518
# not check the method signatures.
519-
queue_interface = [
520-
'empty', 'full', 'get', 'get_nowait',
521-
'put', 'put_nowait', 'join', 'qsize',
522-
'task_done',
523-
]
519+
#
520+
# Note that only 'put_nowait' and 'get' are required by the logging
521+
# queue handler and queue listener (see gh-124653) and that other
522+
# methods are either optional or unused.
523+
minimal_queue_interface = ['put_nowait', 'get']
524524
return all(callable(getattr(obj, method, None))
525-
for method in queue_interface)
525+
for method in minimal_queue_interface)
526526

527527
class DictConfigurator(BaseConfigurator):
528528
"""

‎Lib/test/test_logging.py

Lines changed: 64 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2377,16 +2377,22 @@ def __getattr__(self, attribute):
23772377
return getattr(queue, attribute)
23782378

23792379
class CustomQueueFakeProtocol(CustomQueueProtocol):
2380-
# An object implementing the Queue API (incorrect signatures).
2380+
# An object implementing the minimial Queue API for
2381+
# the logging module but with incorrect signatures.
2382+
#
23812383
# The object will be considered a valid queue class since we
23822384
# do not check the signatures (only callability of methods)
23832385
# but will NOT be usable in production since a TypeError will
2384-
# be raised due to a missing argument.
2385-
def empty(self, x):
2386+
# be raised due to the extra argument in 'put_nowait'.
2387+
def put_nowait(self):
23862388
pass
23872389

23882390
class CustomQueueWrongProtocol(CustomQueueProtocol):
2389-
empty = None
2391+
put_nowait = None
2392+
2393+
class MinimalQueueProtocol:
2394+
def put_nowait(self, x): pass
2395+
def get(self): pass
23902396

23912397
def queueMaker():
23922398
return queue.Queue()
@@ -3946,56 +3952,70 @@ def test_config_queue_handler(self):
39463952
msg = str(ctx.exception)
39473953
self.assertEqual(msg, "Unable to configure handler 'ah'")
39483954

3955+
def _apply_simple_queue_listener_configuration(self, qspec):
3956+
self.apply_config({
3957+
"version": 1,
3958+
"handlers": {
3959+
"queue_listener": {
3960+
"class": "logging.handlers.QueueHandler",
3961+
"queue": qspec,
3962+
},
3963+
},
3964+
})
3965+
39493966
@threading_helper.requires_working_threading()
39503967
@support.requires_subprocess()
39513968
@patch("multiprocessing.Manager")
39523969
def test_config_queue_handler_does_not_create_multiprocessing_manager(self, manager):
3953-
# gh-120868, gh-121723
3954-
3955-
from multiprocessing import Queue as MQ
3956-
3957-
q1 = {"()": "queue.Queue", "maxsize": -1}
3958-
q2 = MQ()
3959-
q3 = queue.Queue()
3960-
# CustomQueueFakeProtocol passes the checks but will not be usable
3961-
# since the signatures are incompatible. Checking the Queue API
3962-
# without testing the type of the actual queue is a trade-off
3963-
# between usability and the work we need to do in order to safely
3964-
# check that the queue object correctly implements the API.
3965-
q4 = CustomQueueFakeProtocol()
3966-
3967-
for qspec in (q1, q2, q3, q4):
3968-
self.apply_config(
3969-
{
3970-
"version": 1,
3971-
"handlers": {
3972-
"queue_listener": {
3973-
"class": "logging.handlers.QueueHandler",
3974-
"queue": qspec,
3975-
},
3976-
},
3977-
}
3978-
)
3979-
manager.assert_not_called()
3970+
# gh-120868, gh-121723, gh-124653
3971+
3972+
for qspec in [
3973+
{"()": "queue.Queue", "maxsize": -1},
3974+
queue.Queue(),
3975+
# queue.SimpleQueue does not inherit from queue.Queue
3976+
queue.SimpleQueue(),
3977+
# CustomQueueFakeProtocol passes the checks but will not be usable
3978+
# since the signatures are incompatible. Checking the Queue API
3979+
# without testing the type of the actual queue is a trade-off
3980+
# between usability and the work we need to do in order to safely
3981+
# check that the queue object correctly implements the API.
3982+
CustomQueueFakeProtocol(),
3983+
MinimalQueueProtocol(),
3984+
]:
3985+
with self.subTest(qspec=qspec):
3986+
self._apply_simple_queue_listener_configuration(qspec)
3987+
manager.assert_not_called()
39803988

39813989
@patch("multiprocessing.Manager")
39823990
def test_config_queue_handler_invalid_config_does_not_create_multiprocessing_manager(self, manager):
39833991
# gh-120868, gh-121723
39843992

39853993
for qspec in [object(), CustomQueueWrongProtocol()]:
3986-
with self.assertRaises(ValueError):
3987-
self.apply_config(
3988-
{
3989-
"version": 1,
3990-
"handlers": {
3991-
"queue_listener": {
3992-
"class": "logging.handlers.QueueHandler",
3993-
"queue": qspec,
3994-
},
3995-
},
3996-
}
3997-
)
3998-
manager.assert_not_called()
3994+
with self.subTest(qspec=qspec), self.assertRaises(ValueError):
3995+
self._apply_simple_queue_listener_configuration(qspec)
3996+
manager.assert_not_called()
3997+
3998+
@skip_if_tsan_fork
3999+
@support.requires_subprocess()
4000+
@unittest.skipUnless(support.Py_DEBUG, "requires a debug build for testing"
4001+
" assertions in multiprocessing")
4002+
def test_config_reject_simple_queue_handler_multiprocessing_context(self):
4003+
# multiprocessing.SimpleQueue does not implement 'put_nowait'
4004+
# and thus cannot be used as a queue-like object (gh-124653)
4005+
4006+
import multiprocessing
4007+
4008+
if support.MS_WINDOWS:
4009+
start_methods = ['spawn']
4010+
else:
4011+
start_methods = ['spawn', 'fork', 'forkserver']
4012+
4013+
for start_method in start_methods:
4014+
with self.subTest(start_method=start_method):
4015+
ctx = multiprocessing.get_context(start_method)
4016+
qspec = ctx.SimpleQueue()
4017+
with self.assertRaises(ValueError):
4018+
self._apply_simple_queue_listener_configuration(qspec)
39994019

40004020
@skip_if_tsan_fork
40014021
@support.requires_subprocess()

‎Misc/NEWS.d/next/Library/2024-10-02-15-05-45.gh-issue-124653.tqsTu9.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix detection of the minimal Queue API needed by the :mod:`logging` module.
2+
Patch by Bénédikt Tran.

0 commit comments

Comments
 (0)
0