mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Replace Pydantic Settings with TypedDict + load_settings() (#3843)
* Replace Pydantic Settings with TypedDict + load_settings() - Remove pydantic-settings dependency, add python-dotenv - Delete _pydantic.py (AFBaseSettings, HTTPsUrl) - Add _settings.py with generic load_settings() function, SecretString, type coercion, and Required field validation (SettingNotFoundError) - Convert all 13 settings classes from AFBaseSettings subclasses to TypedDict definitions with load_settings() calls - Update all consumers from attribute access to dict access - Add 20 unit tests for load_settings() covering basic loading, dotenv, SecretString, type coercion, and required field validation - Update all existing tests for new settings patterns * Fix mypy type errors from settings conversion - Fix str | None attribute access in responses_client (walrus operator) - Fix SecretString | None narrowing in bedrock (type: ignore after guard) - Convert _context_provider.py attribute access to dict access (missed file) - Fix endpoint type narrowing in search_provider and context_provider - Fix purview: str | None .rstrip(), int | None defaults, urlparse bytes * Address PR review: required_fields param, type validation, fixes - Move required field validation from TypedDict annotations (Required) to a required_fields parameter on load_settings(), enabling runtime decisions about which fields are required - Remove Required imports and restore from __future__ import annotations in ollama and foundry_local - Add _check_override_type() for deterministic ServiceInitializationError on invalid override types (e.g. dict passed for str field) - Fix all multi-exception test catches back to single exception type - Fix Ollama host=None: use .get() so None is passed through to SDK default - Fix Purview processor: use explicit is-None checks instead of or operator - Remove unused BaseModel import from openai/_shared.py - Add 4 new tests (24 total): required_fields param, type validation * Fix type validation: allow int for float fields _check_override_type now permits int values for float-typed fields, matching Python's standard numeric promotion behavior. * fix: wrap urlparse arg with str() to fix mypy bytes endswith error
This commit is contained in:
@@ -9,7 +9,7 @@ from ._exceptions import (
|
||||
PurviewServiceError,
|
||||
)
|
||||
from ._middleware import PurviewChatPolicyMiddleware, PurviewPolicyMiddleware
|
||||
from ._settings import PurviewAppLocation, PurviewLocationType, PurviewSettings
|
||||
from ._settings import PurviewAppLocation, PurviewLocationType, PurviewSettings, get_purview_scopes
|
||||
|
||||
__all__ = [
|
||||
"CacheProvider",
|
||||
@@ -23,4 +23,5 @@ __all__ = [
|
||||
"PurviewRequestError",
|
||||
"PurviewServiceError",
|
||||
"PurviewSettings",
|
||||
"get_purview_scopes",
|
||||
]
|
||||
|
||||
@@ -31,7 +31,7 @@ from ._models import (
|
||||
ProtectionScopesRequest,
|
||||
ProtectionScopesResponse,
|
||||
)
|
||||
from ._settings import PurviewSettings
|
||||
from ._settings import PurviewSettings, get_purview_scopes
|
||||
|
||||
logger = get_logger("agent_framework.purview")
|
||||
|
||||
@@ -52,7 +52,7 @@ class PurviewClient:
|
||||
):
|
||||
self._credential: TokenCredential | AsyncTokenCredential = credential
|
||||
self._settings = settings
|
||||
self._graph_uri = settings.graph_base_uri.rstrip("/")
|
||||
self._graph_uri = (settings.get("graph_base_uri") or "https://graph.microsoft.com/v1.0/").rstrip("/")
|
||||
self._timeout = timeout
|
||||
self._client = httpx.AsyncClient(timeout=timeout)
|
||||
|
||||
@@ -61,7 +61,7 @@ class PurviewClient:
|
||||
|
||||
async def _get_token(self, *, tenant_id: str | None = None) -> str:
|
||||
"""Acquire an access token using either async or sync credential."""
|
||||
scopes = self._settings.get_scopes()
|
||||
scopes = get_purview_scopes(self._settings)
|
||||
cred = self._credential
|
||||
token = cred.get_token(*scopes, tenant_id=tenant_id)
|
||||
token = await token if inspect.isawaitable(token) else token
|
||||
@@ -167,7 +167,7 @@ class PurviewClient:
|
||||
if resp.status_code in (401, 403):
|
||||
raise PurviewAuthenticationError(f"Auth failure {resp.status_code}: {resp.text}")
|
||||
if resp.status_code == 402:
|
||||
if self._settings.ignore_payment_required:
|
||||
if self._settings.get("ignore_payment_required", False):
|
||||
return response_type() # type: ignore[call-arg, no-any-return]
|
||||
raise PurviewPaymentRequiredError(f"Payment required {resp.status_code}: {resp.text}")
|
||||
if resp.status_code == 429:
|
||||
|
||||
@@ -78,18 +78,22 @@ class PurviewPolicyMiddleware(AgentMiddleware):
|
||||
from agent_framework import AgentResponse, Message
|
||||
|
||||
context.result = AgentResponse(
|
||||
messages=[Message(role="system", text=self._settings.blocked_prompt_message)]
|
||||
messages=[
|
||||
Message(
|
||||
role="system", text=self._settings.get("blocked_prompt_message", "Prompt blocked by policy")
|
||||
)
|
||||
]
|
||||
)
|
||||
raise MiddlewareTermination
|
||||
except MiddlewareTermination:
|
||||
raise
|
||||
except PurviewPaymentRequiredError as ex:
|
||||
logger.error(f"Purview payment required error in policy pre-check: {ex}")
|
||||
if not self._settings.ignore_payment_required:
|
||||
if not self._settings.get("ignore_payment_required", False):
|
||||
raise
|
||||
except Exception as ex:
|
||||
logger.error(f"Error in Purview policy pre-check: {ex}")
|
||||
if not self._settings.ignore_exceptions:
|
||||
if not self._settings.get("ignore_exceptions", False):
|
||||
raise
|
||||
|
||||
await call_next()
|
||||
@@ -111,18 +115,23 @@ class PurviewPolicyMiddleware(AgentMiddleware):
|
||||
from agent_framework import AgentResponse, Message
|
||||
|
||||
context.result = AgentResponse(
|
||||
messages=[Message(role="system", text=self._settings.blocked_response_message)]
|
||||
messages=[
|
||||
Message(
|
||||
role="system",
|
||||
text=self._settings.get("blocked_response_message", "Response blocked by policy"),
|
||||
)
|
||||
]
|
||||
)
|
||||
else:
|
||||
# Streaming responses are not supported for post-checks
|
||||
logger.debug("Streaming responses are not supported for Purview policy post-checks")
|
||||
except PurviewPaymentRequiredError as ex:
|
||||
logger.error(f"Purview payment required error in policy post-check: {ex}")
|
||||
if not self._settings.ignore_payment_required:
|
||||
if not self._settings.get("ignore_payment_required", False):
|
||||
raise
|
||||
except Exception as ex:
|
||||
logger.error(f"Error in Purview policy post-check: {ex}")
|
||||
if not self._settings.ignore_exceptions:
|
||||
if not self._settings.get("ignore_exceptions", False):
|
||||
raise
|
||||
|
||||
|
||||
@@ -173,18 +182,20 @@ class PurviewChatPolicyMiddleware(ChatMiddleware):
|
||||
if should_block_prompt:
|
||||
from agent_framework import ChatResponse, Message
|
||||
|
||||
blocked_message = Message(role="system", text=self._settings.blocked_prompt_message)
|
||||
blocked_message = Message(
|
||||
role="system", text=self._settings.get("blocked_prompt_message", "Prompt blocked by policy")
|
||||
)
|
||||
context.result = ChatResponse(messages=[blocked_message])
|
||||
raise MiddlewareTermination
|
||||
except MiddlewareTermination:
|
||||
raise
|
||||
except PurviewPaymentRequiredError as ex:
|
||||
logger.error(f"Purview payment required error in policy pre-check: {ex}")
|
||||
if not self._settings.ignore_payment_required:
|
||||
if not self._settings.get("ignore_payment_required", False):
|
||||
raise
|
||||
except Exception as ex:
|
||||
logger.error(f"Error in Purview policy pre-check: {ex}")
|
||||
if not self._settings.ignore_exceptions:
|
||||
if not self._settings.get("ignore_exceptions", False):
|
||||
raise
|
||||
|
||||
await call_next()
|
||||
@@ -205,15 +216,18 @@ class PurviewChatPolicyMiddleware(ChatMiddleware):
|
||||
if should_block_response:
|
||||
from agent_framework import ChatResponse, Message
|
||||
|
||||
blocked_message = Message(role="system", text=self._settings.blocked_response_message)
|
||||
blocked_message = Message(
|
||||
role="system",
|
||||
text=self._settings.get("blocked_response_message", "Response blocked by policy"),
|
||||
)
|
||||
context.result = ChatResponse(messages=[blocked_message])
|
||||
else:
|
||||
logger.debug("Streaming responses are not supported for Purview policy post-checks")
|
||||
except PurviewPaymentRequiredError as ex:
|
||||
logger.error(f"Purview payment required error in policy post-check: {ex}")
|
||||
if not self._settings.ignore_payment_required:
|
||||
if not self._settings.get("ignore_payment_required", False):
|
||||
raise
|
||||
except Exception as ex:
|
||||
logger.error(f"Error in Purview policy post-check: {ex}")
|
||||
if not self._settings.ignore_exceptions:
|
||||
if not self._settings.get("ignore_exceptions", False):
|
||||
raise
|
||||
|
||||
@@ -57,8 +57,11 @@ class ScopedContentProcessor:
|
||||
def __init__(self, client: PurviewClient, settings: PurviewSettings, cache_provider: CacheProvider | None = None):
|
||||
self._client = client
|
||||
self._settings = settings
|
||||
cache_ttl = settings.get("cache_ttl_seconds")
|
||||
max_cache = settings.get("max_cache_size_bytes")
|
||||
self._cache: CacheProvider = cache_provider or InMemoryCacheProvider(
|
||||
default_ttl_seconds=settings.cache_ttl_seconds, max_size_bytes=settings.max_cache_size_bytes
|
||||
default_ttl_seconds=cache_ttl if cache_ttl is not None else 14400,
|
||||
max_size_bytes=max_cache if max_cache is not None else 200 * 1024 * 1024,
|
||||
)
|
||||
self._background_tasks: set[asyncio.Task[Any]] = set()
|
||||
|
||||
@@ -116,10 +119,10 @@ class ScopedContentProcessor:
|
||||
results: list[ProcessContentRequest] = []
|
||||
token_info = None
|
||||
|
||||
if not (self._settings.tenant_id and self._settings.purview_app_location):
|
||||
token_info = await self._client.get_user_info_from_token(tenant_id=self._settings.tenant_id)
|
||||
if not (self._settings.get("tenant_id") and self._settings.get("purview_app_location")):
|
||||
token_info = await self._client.get_user_info_from_token(tenant_id=self._settings.get("tenant_id"))
|
||||
|
||||
tenant_id = (token_info or {}).get("tenant_id") or self._settings.tenant_id
|
||||
tenant_id = (token_info or {}).get("tenant_id") or self._settings.get("tenant_id")
|
||||
if not tenant_id or not _is_valid_guid(tenant_id):
|
||||
raise ValueError("Tenant id required or must be inferable from credential")
|
||||
|
||||
@@ -159,10 +162,11 @@ class ScopedContentProcessor:
|
||||
)
|
||||
activity_meta = ActivityMetadata(activity=activity)
|
||||
|
||||
if self._settings.purview_app_location:
|
||||
purview_app_location = self._settings.get("purview_app_location")
|
||||
if purview_app_location:
|
||||
policy_location = PolicyLocation(
|
||||
data_type=self._settings.purview_app_location.get_policy_location()["@odata.type"],
|
||||
value=self._settings.purview_app_location.location_value,
|
||||
data_type=purview_app_location.get_policy_location()["@odata.type"],
|
||||
value=purview_app_location.location_value,
|
||||
)
|
||||
elif token_info and token_info.get("client_id"):
|
||||
policy_location = PolicyLocation(
|
||||
@@ -172,13 +176,14 @@ class ScopedContentProcessor:
|
||||
else:
|
||||
raise ValueError("App location not provided or inferable")
|
||||
|
||||
app_version = self._settings.app_version or "Unknown"
|
||||
protected_app = ProtectedAppMetadata(
|
||||
name=self._settings.app_name,
|
||||
version=app_version,
|
||||
name=self._settings["app_name"],
|
||||
version=self._settings.get("app_version", "Unknown"),
|
||||
application_location=policy_location,
|
||||
)
|
||||
integrated_app = IntegratedAppMetadata(name=self._settings.app_name, version=app_version)
|
||||
integrated_app = IntegratedAppMetadata(
|
||||
name=self._settings["app_name"], version=self._settings.get("app_version", "Unknown")
|
||||
)
|
||||
device_meta = DeviceMetadata(
|
||||
operating_system_specifications=OperatingSystemSpecifications(
|
||||
operating_system_platform="Unknown", operating_system_version="Unknown"
|
||||
@@ -229,11 +234,13 @@ class ScopedContentProcessor:
|
||||
ps_resp = cached_ps_resp
|
||||
else:
|
||||
try:
|
||||
ttl = self._settings.get("cache_ttl_seconds")
|
||||
ttl_seconds = ttl if ttl is not None else 14400
|
||||
ps_resp = await self._client.get_protection_scopes(ps_req)
|
||||
await self._cache.set(cache_key, ps_resp, ttl_seconds=self._settings.cache_ttl_seconds)
|
||||
await self._cache.set(cache_key, ps_resp, ttl_seconds=ttl_seconds)
|
||||
except PurviewPaymentRequiredError as ex:
|
||||
# Cache the exception at tenant level so all subsequent requests for this tenant fail fast
|
||||
await self._cache.set(tenant_payment_cache_key, ex, ttl_seconds=self._settings.cache_ttl_seconds)
|
||||
await self._cache.set(tenant_payment_cache_key, ex, ttl_seconds=ttl_seconds)
|
||||
raise
|
||||
|
||||
if ps_resp.scope_identifier:
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
import sys
|
||||
from enum import Enum
|
||||
|
||||
from agent_framework._pydantic import AFBaseSettings
|
||||
from pydantic import BaseModel, Field
|
||||
from pydantic_settings import SettingsConfigDict
|
||||
from pydantic import BaseModel
|
||||
|
||||
if sys.version_info >= (3, 11):
|
||||
from typing import TypedDict # pragma: no cover
|
||||
else:
|
||||
from typing_extensions import TypedDict # type: ignore # pragma: no cover
|
||||
|
||||
|
||||
class PurviewLocationType(str, Enum):
|
||||
@@ -18,8 +22,8 @@ class PurviewLocationType(str, Enum):
|
||||
class PurviewAppLocation(BaseModel):
|
||||
"""Identifier representing the app's location for Purview policy evaluation."""
|
||||
|
||||
location_type: PurviewLocationType = Field(..., description="The location type.")
|
||||
location_value: str = Field(..., description="The location value.")
|
||||
location_type: PurviewLocationType
|
||||
location_value: str
|
||||
|
||||
def get_policy_location(self) -> dict[str, str]:
|
||||
ns = "microsoft.graph"
|
||||
@@ -34,8 +38,8 @@ class PurviewAppLocation(BaseModel):
|
||||
return {"@odata.type": dt, "value": self.location_value}
|
||||
|
||||
|
||||
class PurviewSettings(AFBaseSettings):
|
||||
"""Settings for Purview integration.
|
||||
class PurviewSettings(TypedDict, total=False):
|
||||
"""Settings for Purview integration mirroring .NET PurviewSettings.
|
||||
|
||||
Attributes:
|
||||
app_name: Public app name.
|
||||
@@ -51,40 +55,30 @@ class PurviewSettings(AFBaseSettings):
|
||||
max_cache_size_bytes: Maximum cache size in bytes (default 200MB).
|
||||
"""
|
||||
|
||||
app_name: str = Field(...)
|
||||
app_version: str | None = Field(default=None)
|
||||
tenant_id: str | None = Field(default=None)
|
||||
purview_app_location: PurviewAppLocation | None = Field(default=None)
|
||||
graph_base_uri: str = Field(default="https://graph.microsoft.com/v1.0/")
|
||||
blocked_prompt_message: str = Field(
|
||||
default="Prompt blocked by policy",
|
||||
description="Message to return when a prompt is blocked by policy.",
|
||||
)
|
||||
blocked_response_message: str = Field(
|
||||
default="Response blocked by policy",
|
||||
description="Message to return when a response is blocked by policy.",
|
||||
)
|
||||
ignore_exceptions: bool = Field(
|
||||
default=False,
|
||||
description="If True, all Purview exceptions will be logged but not thrown in middleware.",
|
||||
)
|
||||
ignore_payment_required: bool = Field(
|
||||
default=False,
|
||||
description="If True, 402 payment required errors will be logged but not thrown.",
|
||||
)
|
||||
cache_ttl_seconds: int = Field(
|
||||
default=14400,
|
||||
description="Time to live for cache entries in seconds (default 14400 = 4 hours).",
|
||||
)
|
||||
max_cache_size_bytes: int = Field(
|
||||
default=200 * 1024 * 1024,
|
||||
description="Maximum cache size in bytes (default 200MB).",
|
||||
)
|
||||
app_name: str | None
|
||||
app_version: str | None
|
||||
tenant_id: str | None
|
||||
purview_app_location: PurviewAppLocation | None
|
||||
graph_base_uri: str | None
|
||||
blocked_prompt_message: str | None
|
||||
blocked_response_message: str | None
|
||||
ignore_exceptions: bool | None
|
||||
ignore_payment_required: bool | None
|
||||
cache_ttl_seconds: int | None
|
||||
max_cache_size_bytes: int | None
|
||||
|
||||
model_config = SettingsConfigDict(populate_by_name=True, validate_assignment=True)
|
||||
|
||||
def get_scopes(self) -> list[str]:
|
||||
from urllib.parse import urlparse
|
||||
def get_purview_scopes(settings: PurviewSettings) -> list[str]:
|
||||
"""Get the OAuth scopes for the Purview Graph API.
|
||||
|
||||
host = urlparse(self.graph_base_uri).hostname or "graph.microsoft.com"
|
||||
return [f"https://{host}/.default"]
|
||||
Args:
|
||||
settings: The Purview settings containing graph_base_uri.
|
||||
|
||||
Returns:
|
||||
A list of OAuth scope strings.
|
||||
"""
|
||||
from urllib.parse import urlparse
|
||||
|
||||
graph_base_uri = settings.get("graph_base_uri", "https://graph.microsoft.com/v1.0/")
|
||||
host = urlparse(str(graph_base_uri)).hostname or "graph.microsoft.com"
|
||||
return [f"https://{host}/.default"]
|
||||
|
||||
@@ -125,7 +125,7 @@ class TestPurviewChatPolicyMiddleware:
|
||||
) -> None:
|
||||
"""Test that exceptions in post-check are logged but don't affect result when ignore_exceptions=True."""
|
||||
# Set ignore_exceptions to True to test exception suppression
|
||||
middleware._settings.ignore_exceptions = True
|
||||
middleware._settings["ignore_exceptions"] = True
|
||||
|
||||
call_count = 0
|
||||
|
||||
|
||||
@@ -119,7 +119,7 @@ class TestPurviewPolicyMiddleware:
|
||||
) -> None:
|
||||
"""Test middleware handles result that doesn't have messages attribute."""
|
||||
# Set ignore_exceptions to True so AttributeError is caught and logged
|
||||
middleware._settings.ignore_exceptions = True
|
||||
middleware._settings["ignore_exceptions"] = True
|
||||
|
||||
context = AgentContext(agent=mock_agent, messages=[Message(role="user", text="Hello")])
|
||||
|
||||
@@ -216,7 +216,7 @@ class TestPurviewPolicyMiddleware:
|
||||
self, middleware: PurviewPolicyMiddleware, mock_agent: MagicMock
|
||||
) -> None:
|
||||
"""Test that post-check exceptions are propagated when ignore_exceptions=False."""
|
||||
middleware._settings.ignore_exceptions = False
|
||||
middleware._settings["ignore_exceptions"] = False
|
||||
|
||||
context = AgentContext(agent=mock_agent, messages=[Message(role="user", text="Hello")])
|
||||
|
||||
@@ -242,7 +242,7 @@ class TestPurviewPolicyMiddleware:
|
||||
) -> None:
|
||||
"""Test that exceptions in pre-check are logged but don't stop processing when ignore_exceptions=True."""
|
||||
# Set ignore_exceptions to True
|
||||
middleware._settings.ignore_exceptions = True
|
||||
middleware._settings["ignore_exceptions"] = True
|
||||
|
||||
context = AgentContext(agent=mock_agent, messages=[Message(role="user", text="Test")])
|
||||
|
||||
@@ -265,7 +265,7 @@ class TestPurviewPolicyMiddleware:
|
||||
) -> None:
|
||||
"""Test that exceptions in post-check are logged but don't affect result when ignore_exceptions=True."""
|
||||
# Set ignore_exceptions to True
|
||||
middleware._settings.ignore_exceptions = True
|
||||
middleware._settings["ignore_exceptions"] = True
|
||||
|
||||
context = AgentContext(agent=mock_agent, messages=[Message(role="user", text="Test")])
|
||||
|
||||
|
||||
@@ -636,7 +636,6 @@ class TestScopedContentProcessorCaching:
|
||||
return PurviewSettings(
|
||||
app_name="Test App",
|
||||
tenant_id="12345678-1234-1234-1234-123456789012",
|
||||
default_user_id="12345678-1234-1234-1234-123456789012",
|
||||
purview_app_location=location,
|
||||
)
|
||||
|
||||
|
||||
@@ -47,7 +47,7 @@ class TestPurviewClient:
|
||||
@pytest.fixture
|
||||
def settings(self) -> PurviewSettings:
|
||||
"""Create test settings."""
|
||||
return PurviewSettings(app_name="Test App", tenant_id="test-tenant", default_user_id="test-user")
|
||||
return PurviewSettings(app_name="Test App", tenant_id="test-tenant")
|
||||
|
||||
@pytest.fixture
|
||||
async def client(
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
|
||||
import pytest
|
||||
|
||||
from agent_framework_purview import PurviewAppLocation, PurviewLocationType, PurviewSettings
|
||||
from agent_framework_purview import PurviewAppLocation, PurviewLocationType, PurviewSettings, get_purview_scopes
|
||||
|
||||
|
||||
class TestPurviewSettings:
|
||||
@@ -14,10 +14,10 @@ class TestPurviewSettings:
|
||||
"""Test PurviewSettings with default values."""
|
||||
settings = PurviewSettings(app_name="Test App")
|
||||
|
||||
assert settings.app_name == "Test App"
|
||||
assert settings.graph_base_uri == "https://graph.microsoft.com/v1.0/"
|
||||
assert settings.tenant_id is None
|
||||
assert settings.purview_app_location is None
|
||||
assert settings["app_name"] == "Test App"
|
||||
assert settings.get("graph_base_uri") is None
|
||||
assert settings.get("tenant_id") is None
|
||||
assert settings.get("purview_app_location") is None
|
||||
|
||||
def test_settings_with_custom_values(self) -> None:
|
||||
"""Test PurviewSettings with custom values."""
|
||||
@@ -30,9 +30,9 @@ class TestPurviewSettings:
|
||||
purview_app_location=app_location,
|
||||
)
|
||||
|
||||
assert settings.graph_base_uri == "https://graph.microsoft-ppe.com"
|
||||
assert settings.tenant_id == "test-tenant-id"
|
||||
assert settings.purview_app_location.location_value == "app-123"
|
||||
assert settings["graph_base_uri"] == "https://graph.microsoft-ppe.com"
|
||||
assert settings["tenant_id"] == "test-tenant-id"
|
||||
assert settings["purview_app_location"].location_value == "app-123"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"graph_uri,expected_scope",
|
||||
@@ -44,7 +44,7 @@ class TestPurviewSettings:
|
||||
def test_get_scopes(self, graph_uri: str, expected_scope: str) -> None:
|
||||
"""Test get_scopes returns correct scope for different URIs."""
|
||||
settings = PurviewSettings(app_name="Test App", graph_base_uri=graph_uri)
|
||||
scopes = settings.get_scopes()
|
||||
scopes = get_purview_scopes(settings)
|
||||
|
||||
assert len(scopes) == 1
|
||||
assert expected_scope in scopes
|
||||
|
||||
Reference in New Issue
Block a user