mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-15 23:31:29 +00:00
fix: skip V1 legacy email path when enterprise email already sent
Make V1 a proper fallback: track whether enterprise (V2) email loggers dispatched the invitation, and only fire the legacy V1 path when they did not. Prevents a ValueError when enterprise loggers are registered but "email" is not in general_settings["alerting"]. Addresses Greptile review feedback on #26205. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
4a3c69c43d
commit
a2effa0ef5
2 changed files with 97 additions and 10 deletions
|
|
@ -178,6 +178,7 @@ class UserManagementEventHooks:
|
|||
)
|
||||
use_enterprise_email_hooks = False
|
||||
|
||||
enterprise_email_sent = False
|
||||
if use_enterprise_email_hooks and should_send_email:
|
||||
initialized_email_loggers = litellm.logging_callback_manager.get_custom_loggers_for_type(
|
||||
callback_type=BaseEmailLogger # type: ignore
|
||||
|
|
@ -188,11 +189,12 @@ class UserManagementEventHooks:
|
|||
await email_logger.send_user_invitation_email( # type: ignore
|
||||
event=event,
|
||||
)
|
||||
enterprise_email_sent = True
|
||||
|
||||
#########################################################
|
||||
########## LEGACY V1 USER INVITATION EMAIL ################
|
||||
#########################################################
|
||||
if should_send_email:
|
||||
if should_send_email and not enterprise_email_sent:
|
||||
await UserManagementEventHooks.send_legacy_v1_user_invitation_email(
|
||||
data=data,
|
||||
response=response,
|
||||
|
|
|
|||
|
|
@ -2,11 +2,18 @@ import pytest
|
|||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
from litellm.proxy.hooks.user_management_event_hooks import UserManagementEventHooks
|
||||
from litellm.proxy.hooks.key_management_event_hooks import KeyManagementEventHooks
|
||||
from litellm.proxy._types import NewUserRequest, NewUserResponse, GenerateKeyRequest, GenerateKeyResponse, UserAPIKeyAuth
|
||||
from litellm.proxy._types import (
|
||||
NewUserRequest,
|
||||
NewUserResponse,
|
||||
GenerateKeyRequest,
|
||||
GenerateKeyResponse,
|
||||
UserAPIKeyAuth,
|
||||
)
|
||||
import builtins
|
||||
import sys
|
||||
from types import SimpleNamespace
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_v1_user_creation_no_email_when_send_invite_email_false():
|
||||
"""
|
||||
|
|
@ -17,7 +24,9 @@ async def test_v1_user_creation_no_email_when_send_invite_email_false():
|
|||
mock_proxy_logging_obj = MagicMock()
|
||||
mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting
|
||||
|
||||
with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]):
|
||||
with patch(
|
||||
"litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]
|
||||
):
|
||||
mock_proxy_server = SimpleNamespace(
|
||||
general_settings={"alerting": ["email"]},
|
||||
proxy_logging_obj=mock_proxy_logging_obj,
|
||||
|
|
@ -43,6 +52,7 @@ async def test_v1_user_creation_no_email_when_send_invite_email_false():
|
|||
)
|
||||
mock_slack_alerting.send_key_created_or_user_invited_email.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_v1_user_creation_sends_email_when_send_invite_email_true():
|
||||
"""
|
||||
|
|
@ -53,7 +63,9 @@ async def test_v1_user_creation_sends_email_when_send_invite_email_true():
|
|||
mock_proxy_logging_obj = MagicMock()
|
||||
mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting
|
||||
|
||||
with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]):
|
||||
with patch(
|
||||
"litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]
|
||||
):
|
||||
mock_proxy_server = SimpleNamespace(
|
||||
general_settings={"alerting": ["email"]},
|
||||
proxy_logging_obj=mock_proxy_logging_obj,
|
||||
|
|
@ -79,6 +91,7 @@ async def test_v1_user_creation_sends_email_when_send_invite_email_true():
|
|||
)
|
||||
mock_slack_alerting.send_key_created_or_user_invited_email.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_v1_key_generation_sends_email_when_send_invite_email_true():
|
||||
"""
|
||||
|
|
@ -90,14 +103,21 @@ async def test_v1_key_generation_sends_email_when_send_invite_email_true():
|
|||
mock_proxy_logging_obj = MagicMock()
|
||||
mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting
|
||||
|
||||
with patch.object(KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email):
|
||||
with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]):
|
||||
with patch.object(
|
||||
KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email
|
||||
):
|
||||
with patch(
|
||||
"litellm.logging_callback_manager.get_custom_loggers_for_type",
|
||||
return_value=[],
|
||||
):
|
||||
mock_proxy_server = SimpleNamespace(
|
||||
general_settings={"alerting": ["email"]},
|
||||
proxy_logging_obj=mock_proxy_logging_obj,
|
||||
litellm_proxy_admin_name="admin-user",
|
||||
)
|
||||
with patch.dict(sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}):
|
||||
with patch.dict(
|
||||
sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}
|
||||
):
|
||||
data = GenerateKeyRequest(
|
||||
user_email="test@example.com",
|
||||
send_invite_email=True, # Should send key email
|
||||
|
|
@ -116,6 +136,7 @@ async def test_v1_key_generation_sends_email_when_send_invite_email_true():
|
|||
)
|
||||
mock_send_key_created_email.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_v1_key_generation_no_email_when_send_invite_email_false():
|
||||
"""
|
||||
|
|
@ -127,14 +148,21 @@ async def test_v1_key_generation_no_email_when_send_invite_email_false():
|
|||
mock_proxy_logging_obj = MagicMock()
|
||||
mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting
|
||||
|
||||
with patch.object(KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email):
|
||||
with patch("litellm.logging_callback_manager.get_custom_loggers_for_type", return_value=[]):
|
||||
with patch.object(
|
||||
KeyManagementEventHooks, "_send_key_created_email", mock_send_key_created_email
|
||||
):
|
||||
with patch(
|
||||
"litellm.logging_callback_manager.get_custom_loggers_for_type",
|
||||
return_value=[],
|
||||
):
|
||||
mock_proxy_server = SimpleNamespace(
|
||||
general_settings={"alerting": ["email"]},
|
||||
proxy_logging_obj=mock_proxy_logging_obj,
|
||||
litellm_proxy_admin_name="admin-user",
|
||||
)
|
||||
with patch.dict(sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}):
|
||||
with patch.dict(
|
||||
sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}
|
||||
):
|
||||
data = GenerateKeyRequest(
|
||||
user_email="test@example.com",
|
||||
send_invite_email=False, # Should NOT send key email
|
||||
|
|
@ -266,3 +294,60 @@ async def test_v1_user_creation_no_email_when_send_invite_email_none_and_no_user
|
|||
user_api_key_dict=user_api_key_dict,
|
||||
)
|
||||
mock_slack_alerting.send_key_created_or_user_invited_email.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_v1_user_creation_no_legacy_email_when_enterprise_handles_send():
|
||||
"""
|
||||
When enterprise email loggers are registered and send the email,
|
||||
the legacy V1 path should NOT also fire (which would raise ValueError
|
||||
if "email" is not in general_settings["alerting"]).
|
||||
"""
|
||||
mock_slack_alerting = MagicMock()
|
||||
mock_slack_alerting.send_key_created_or_user_invited_email = AsyncMock()
|
||||
mock_proxy_logging_obj = MagicMock()
|
||||
mock_proxy_logging_obj.slack_alerting_instance = mock_slack_alerting
|
||||
|
||||
mock_email_logger = MagicMock()
|
||||
mock_email_logger.send_user_invitation_email = AsyncMock()
|
||||
|
||||
mock_proxy_server = SimpleNamespace(
|
||||
general_settings={},
|
||||
proxy_logging_obj=mock_proxy_logging_obj,
|
||||
litellm_proxy_admin_name="admin-user",
|
||||
)
|
||||
|
||||
with patch.dict(sys.modules, {"litellm.proxy.proxy_server": mock_proxy_server}):
|
||||
with patch(
|
||||
"litellm.logging_callback_manager.get_custom_loggers_for_type",
|
||||
return_value=[mock_email_logger],
|
||||
):
|
||||
with patch.dict(
|
||||
sys.modules,
|
||||
{
|
||||
"litellm_enterprise": MagicMock(),
|
||||
"litellm_enterprise.enterprise_callbacks": MagicMock(),
|
||||
"litellm_enterprise.enterprise_callbacks.send_emails": MagicMock(),
|
||||
"litellm_enterprise.enterprise_callbacks.send_emails.base_email": MagicMock(
|
||||
BaseEmailLogger=type(mock_email_logger),
|
||||
),
|
||||
},
|
||||
):
|
||||
data = NewUserRequest(
|
||||
user_email="test@example.com",
|
||||
)
|
||||
response = NewUserResponse(
|
||||
user_id="test-user",
|
||||
user_email="test@example.com",
|
||||
key="sk-test-key",
|
||||
)
|
||||
user_api_key_dict = UserAPIKeyAuth(
|
||||
user_id="admin-user", api_key="admin-key"
|
||||
)
|
||||
await UserManagementEventHooks.async_send_user_invitation_email(
|
||||
data=data,
|
||||
response=response,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
)
|
||||
mock_email_logger.send_user_invitation_email.assert_called_once()
|
||||
mock_slack_alerting.send_key_created_or_user_invited_email.assert_not_called()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue