mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: improve .env handling and observability samples (#4032)
* Python: improve .env precedence and observability samples - Switch load_settings to explicit precedence: overrides -> explicit .env -> environment -> defaults\n- Raise when env_file_path is provided but missing\n- Update settings docs and tests for new behavior\n- Refresh observability samples and README guidance for env loading options\n\nCloses #3864\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fixed some imports * Fix load_settings CI regressions Allow explicit env_file_path values that exist but are not regular files (for example /dev/null) by checking path existence before dotenv parsing, and restore a dict accumulator with typed return cast to satisfy mypy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Avoid implicit dotenv in observability Only load dotenv in observability helpers when env_file_path is explicitly provided, and remove test os.devnull workarounds that are no longer necessary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
f900febb6f
commit
534e5f5bf7
@@ -130,7 +130,7 @@ def test_azure_assistants_client_init_missing_deployment_name(azure_openai_unit_
|
||||
"""Test AzureOpenAIAssistantsClient initialization with missing deployment name."""
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
AzureOpenAIAssistantsClient(
|
||||
api_key=azure_openai_unit_test_env.get("AZURE_OPENAI_API_KEY", "test-key"), env_file_path="nonexistent.env"
|
||||
api_key=azure_openai_unit_test_env.get("AZURE_OPENAI_API_KEY", "test-key")
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -95,7 +95,6 @@ def test_init_endpoint(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
def test_init_with_empty_deployment_name(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
AzureOpenAIChatClient(
|
||||
env_file_path="test.env",
|
||||
)
|
||||
|
||||
|
||||
@@ -103,7 +102,6 @@ def test_init_with_empty_deployment_name(azure_openai_unit_test_env: dict[str, s
|
||||
def test_init_with_empty_endpoint_and_base_url(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
AzureOpenAIChatClient(
|
||||
env_file_path="test.env",
|
||||
)
|
||||
|
||||
|
||||
@@ -126,7 +124,6 @@ def test_serialize(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
"api_key": azure_openai_unit_test_env["AZURE_OPENAI_API_KEY"],
|
||||
"api_version": azure_openai_unit_test_env["AZURE_OPENAI_API_VERSION"],
|
||||
"default_headers": default_headers,
|
||||
"env_file_path": "test.env",
|
||||
}
|
||||
|
||||
azure_chat_client = AzureOpenAIChatClient.from_dict(settings)
|
||||
|
||||
@@ -112,7 +112,6 @@ def test_init_with_default_header(azure_openai_unit_test_env: dict[str, str]) ->
|
||||
def test_init_with_empty_model_id(azure_openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
AzureOpenAIResponsesClient(
|
||||
env_file_path="test.env",
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -61,7 +61,7 @@ def span_exporter(monkeypatch, enable_instrumentation: bool, enable_sensitive_da
|
||||
importlib.reload(observability)
|
||||
|
||||
# recreate observability settings with values from above and no file.
|
||||
observability_settings = observability.ObservabilitySettings(env_file_path="test.env")
|
||||
observability_settings = observability.ObservabilitySettings()
|
||||
|
||||
# Configure providers manually without calling _configure() to avoid OTLP imports
|
||||
if enable_instrumentation or enable_sensitive_data:
|
||||
|
||||
@@ -901,7 +901,7 @@ def test_console_exporters_opt_in_false(monkeypatch):
|
||||
monkeypatch.setenv("ENABLE_CONSOLE_EXPORTERS", "false")
|
||||
monkeypatch.delenv("OTEL_EXPORTER_OTLP_ENDPOINT", raising=False)
|
||||
|
||||
settings = ObservabilitySettings(env_file_path="test.env")
|
||||
settings = ObservabilitySettings()
|
||||
assert settings.enable_console_exporters is False
|
||||
|
||||
|
||||
@@ -911,7 +911,7 @@ def test_console_exporters_opt_in_true(monkeypatch):
|
||||
|
||||
monkeypatch.setenv("ENABLE_CONSOLE_EXPORTERS", "true")
|
||||
|
||||
settings = ObservabilitySettings(env_file_path="test.env")
|
||||
settings = ObservabilitySettings()
|
||||
assert settings.enable_console_exporters is True
|
||||
|
||||
|
||||
@@ -921,7 +921,7 @@ def test_console_exporters_default_false(monkeypatch):
|
||||
|
||||
monkeypatch.delenv("ENABLE_CONSOLE_EXPORTERS", raising=False)
|
||||
|
||||
settings = ObservabilitySettings(env_file_path="test.env")
|
||||
settings = ObservabilitySettings()
|
||||
assert settings.enable_console_exporters is False
|
||||
|
||||
|
||||
@@ -996,7 +996,7 @@ def test_observability_settings_is_setup_initial(monkeypatch):
|
||||
from agent_framework.observability import ObservabilitySettings
|
||||
|
||||
monkeypatch.delenv("ENABLE_INSTRUMENTATION", raising=False)
|
||||
settings = ObservabilitySettings(env_file_path="test.env")
|
||||
settings = ObservabilitySettings()
|
||||
assert settings.is_setup is False
|
||||
|
||||
|
||||
@@ -1464,7 +1464,7 @@ def test_observability_settings_configure_not_enabled(monkeypatch):
|
||||
from agent_framework.observability import ObservabilitySettings
|
||||
|
||||
monkeypatch.setenv("ENABLE_INSTRUMENTATION", "false")
|
||||
settings = ObservabilitySettings(env_file_path="test.env")
|
||||
settings = ObservabilitySettings()
|
||||
|
||||
# Should not raise, should just return early
|
||||
settings._configure()
|
||||
@@ -1485,7 +1485,7 @@ def test_observability_settings_configure_already_setup(monkeypatch):
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
settings = ObservabilitySettings(env_file_path="test.env")
|
||||
settings = ObservabilitySettings()
|
||||
|
||||
# Manually mark as set up
|
||||
settings._executed_setup = True
|
||||
@@ -2021,7 +2021,7 @@ def test_configure_providers_with_span_exporters(monkeypatch):
|
||||
]:
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
settings = ObservabilitySettings(env_file_path="test.env")
|
||||
settings = ObservabilitySettings()
|
||||
|
||||
# Create mock span exporter
|
||||
mock_span_exporter = Mock(spec=SpanExporter)
|
||||
|
||||
@@ -8,7 +8,7 @@ from typing import TypedDict
|
||||
|
||||
import pytest
|
||||
|
||||
from agent_framework._settings import SecretString, load_settings
|
||||
from agent_framework import SecretString, load_settings
|
||||
|
||||
|
||||
class SimpleSettings(TypedDict, total=False):
|
||||
@@ -106,7 +106,7 @@ class TestDotenvFile:
|
||||
finally:
|
||||
os.unlink(env_path)
|
||||
|
||||
def test_env_vars_override_dotenv(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_dotenv_overrides_env_vars_when_env_file_path_is_set(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("TEST_APP_API_KEY", "real-env-key")
|
||||
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False) as f:
|
||||
@@ -117,15 +117,34 @@ class TestDotenvFile:
|
||||
try:
|
||||
settings = load_settings(SimpleSettings, env_prefix="TEST_APP_", env_file_path=env_path)
|
||||
|
||||
assert settings["api_key"] == "real-env-key"
|
||||
assert settings["api_key"] == "dotenv-key"
|
||||
finally:
|
||||
os.unlink(env_path)
|
||||
|
||||
def test_missing_dotenv_file(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("TEST_APP_API_KEY", raising=False)
|
||||
settings = load_settings(SimpleSettings, env_prefix="TEST_APP_", env_file_path="/nonexistent/.env")
|
||||
def test_env_vars_are_used_when_env_file_path_is_not_set(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("TEST_APP_API_KEY", "real-env-key")
|
||||
settings = load_settings(SimpleSettings, env_prefix="TEST_APP_")
|
||||
|
||||
assert settings["api_key"] is None
|
||||
assert settings["api_key"] == "real-env-key"
|
||||
|
||||
def test_overrides_beat_dotenv_and_env_vars(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("TEST_APP_TIMEOUT", "120")
|
||||
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False) as f:
|
||||
f.write("TEST_APP_TIMEOUT=90\n")
|
||||
f.flush()
|
||||
env_path = f.name
|
||||
|
||||
try:
|
||||
settings = load_settings(SimpleSettings, env_prefix="TEST_APP_", env_file_path=env_path, timeout=60)
|
||||
|
||||
assert settings["timeout"] == 60
|
||||
finally:
|
||||
os.unlink(env_path)
|
||||
|
||||
def test_missing_dotenv_file_raises(self) -> None:
|
||||
with pytest.raises(FileNotFoundError):
|
||||
load_settings(SimpleSettings, env_prefix="TEST_APP_", env_file_path="/nonexistent/.env")
|
||||
|
||||
|
||||
class TestSecretString:
|
||||
|
||||
@@ -155,7 +155,7 @@ def test_init_missing_model_id(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Test OpenAIAssistantsClient initialization with missing model ID."""
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
OpenAIAssistantsClient(
|
||||
api_key=openai_unit_test_env.get("OPENAI_API_KEY", "test-key"), env_file_path="nonexistent.env"
|
||||
api_key=openai_unit_test_env.get("OPENAI_API_KEY", "test-key")
|
||||
)
|
||||
|
||||
|
||||
@@ -163,7 +163,7 @@ def test_init_missing_model_id(openai_unit_test_env: dict[str, str]) -> None:
|
||||
def test_init_missing_api_key(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Test OpenAIAssistantsClient initialization with missing API key."""
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
OpenAIAssistantsClient(model_id="gpt-4", env_file_path="nonexistent.env")
|
||||
OpenAIAssistantsClient(model_id="gpt-4")
|
||||
|
||||
|
||||
def test_init_with_default_headers(openai_unit_test_env: dict[str, str]) -> None:
|
||||
|
||||
@@ -98,7 +98,6 @@ def test_init_base_url_from_settings_env() -> None:
|
||||
def test_init_with_empty_model_id(openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
OpenAIChatClient(
|
||||
env_file_path="test.env",
|
||||
)
|
||||
|
||||
|
||||
@@ -109,7 +108,6 @@ def test_init_with_empty_api_key(openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
OpenAIChatClient(
|
||||
model_id=model_id,
|
||||
env_file_path="test.env",
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -140,7 +140,6 @@ def test_init_with_default_header(openai_unit_test_env: dict[str, str]) -> None:
|
||||
def test_init_with_empty_model_id(openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
OpenAIResponsesClient(
|
||||
env_file_path="test.env",
|
||||
)
|
||||
|
||||
|
||||
@@ -151,7 +150,6 @@ def test_init_with_empty_api_key(openai_unit_test_env: dict[str, str]) -> None:
|
||||
with pytest.raises(ServiceInitializationError):
|
||||
OpenAIResponsesClient(
|
||||
model_id=model_id,
|
||||
env_file_path="test.env",
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user