mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-28 06:23:07 +00:00
fix(utils): set up loggers after releasing the callback lists lock
function_setup held the callback lists lock while set_callbacks ran its one-time logger setup, which can block on a Slack package install, so other threads and Router.discard waited on it. The lock now covers only the list reads and writes.
This commit is contained in:
parent
68a4511d66
commit
2e9bb5f610
2 changed files with 63 additions and 8 deletions
|
|
@ -1152,14 +1152,13 @@ def function_setup(
|
|||
f"Initialized litellm callbacks, Async Success Callbacks: {litellm._async_success_callback}"
|
||||
)
|
||||
|
||||
if (
|
||||
len(litellm.input_callback) > 0
|
||||
or len(litellm.success_callback) > 0
|
||||
or len(litellm.failure_callback) > 0
|
||||
) and len(callback_list) == 0:
|
||||
callback_list = list(set(litellm.input_callback + litellm.success_callback + litellm.failure_callback))
|
||||
get_set_callbacks: Final[Callable[[], Callable[..., None]]] = litellm_utils.get_set_callbacks
|
||||
get_set_callbacks()(callback_list=callback_list, function_id=function_id)
|
||||
callbacks_to_set_up: Final = (
|
||||
list(set(litellm.input_callback + litellm.success_callback + litellm.failure_callback))
|
||||
if len(callback_list) == 0
|
||||
else []
|
||||
)
|
||||
if callbacks_to_set_up:
|
||||
callback_list = callbacks_to_set_up
|
||||
## ASYNC CALLBACKS - safety net for callbacks added via direct append
|
||||
if len(litellm.input_callback) > 0:
|
||||
removed_async_items = []
|
||||
|
|
@ -1201,6 +1200,9 @@ def function_setup(
|
|||
# Pop the async items from failure_callback in reverse order to avoid index issues
|
||||
for index in reversed(removed_async_items):
|
||||
litellm.failure_callback.pop(index)
|
||||
if callbacks_to_set_up:
|
||||
get_set_callbacks: Final[Callable[[], Callable[..., None]]] = litellm_utils.get_set_callbacks
|
||||
get_set_callbacks()(callback_list=callbacks_to_set_up, function_id=function_id)
|
||||
### DYNAMIC CALLBACKS ###
|
||||
dynamic_success_callbacks: list[str | Callable[..., object] | CustomLogger] | None = None
|
||||
dynamic_async_success_callbacks: list[str | Callable[..., object] | CustomLogger] | None = None
|
||||
|
|
|
|||
|
|
@ -3935,6 +3935,59 @@ def test_function_setup_moves_async_callbacks_while_another_thread_drops_a_callb
|
|||
assert litellm._async_success_callback == [async_callback]
|
||||
|
||||
|
||||
class _DropsACallbackFromAnotherThreadWhenComparedToSentry:
|
||||
def __init__(self, dropped: Callable[..., object]) -> None:
|
||||
self.dropped = dropped
|
||||
self.drop_thread: threading.Thread | None = None
|
||||
self.drop_finished_during_logger_setup = False
|
||||
|
||||
def __eq__(self, other: object) -> bool:
|
||||
if isinstance(other, str) and other == "sentry" and self.drop_thread is None:
|
||||
self.drop_thread = threading.Thread(
|
||||
target=litellm.logging_callback_manager.drop_callbacks, args=((self.dropped,),)
|
||||
)
|
||||
self.drop_thread.start()
|
||||
self.drop_thread.join(timeout=5)
|
||||
self.drop_finished_during_logger_setup = not self.drop_thread.is_alive()
|
||||
return self is other
|
||||
|
||||
__hash__ = object.__hash__
|
||||
|
||||
|
||||
def test_function_setup_lets_other_threads_drop_callbacks_while_it_sets_up_loggers(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
def dropped_callback(*args: object, **kwargs: object) -> None:
|
||||
return None
|
||||
|
||||
drops_on_compare: Final = _DropsACallbackFromAnotherThreadWhenComparedToSentry(dropped_callback)
|
||||
monkeypatch.setattr(litellm.utils, "callback_list", [])
|
||||
for list_name in (
|
||||
"callbacks",
|
||||
"input_callback",
|
||||
"failure_callback",
|
||||
"service_callback",
|
||||
"_async_input_callback",
|
||||
"_async_success_callback",
|
||||
"_async_failure_callback",
|
||||
):
|
||||
monkeypatch.setattr(litellm, list_name, [])
|
||||
monkeypatch.setattr(litellm, "success_callback", [drops_on_compare])
|
||||
|
||||
litellm.utils.function_setup(
|
||||
"completion",
|
||||
litellm.utils.Rules(),
|
||||
datetime.now(),
|
||||
model="gpt-4o",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
litellm_call_id="logger-setup-outside-lock",
|
||||
)
|
||||
assert drops_on_compare.drop_thread is not None
|
||||
drops_on_compare.drop_thread.join()
|
||||
|
||||
assert drops_on_compare.drop_finished_during_logger_setup
|
||||
|
||||
|
||||
class TestMetadataNoneHandling:
|
||||
"""
|
||||
Test that metadata=None in kwargs doesn't cause TypeError.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue