Fix critical PR review issues
Fixed high-impact issues from PR #1024 code review: 1. **Boolean conversion bug (schema.py)** - Fixed _expand_env_vars returning strings 'true'/'false' instead of booleans - Now properly converts boolean-like strings (true/false/1/0/yes/no/on/off) to actual booleans - Simplified logic by removing redundant string-to-string conversions - Added support for common boolean string variations 2. **Dependency management (pyproject.toml)** - Removed pytest from main dependencies (now only in dev dependencies) - Moved azure-identity to optional dependencies under new [azure] group - Prevents forcing Azure and testing dependencies on all users 3. **Conditional Azure imports (utils.py)** - Made azure-identity import conditional in create_azure_credential_token_provider() - Raises helpful ImportError with installation instructions if not available - Follows lazy-import pattern for optional dependencies 4. **Documentation fix (graphiti_mcp_server.py)** - Fixed confusing JSON escaping in add_memory docstring example - Changed from triple-backslash escaping to standard JSON string - Updated comment to clarify standard JSON escaping is used Issues verified as already fixed: - Docker build context (all docker-compose files use context: ..) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
ec1066c4f0
commit
7520c8f8d7
4 changed files with 31 additions and 23 deletions
|
|
@ -8,13 +8,14 @@ dependencies = [
|
|||
"mcp>=1.9.4",
|
||||
"openai>=1.91.0",
|
||||
"graphiti-core[falkordb]>=0.16.0",
|
||||
"azure-identity>=1.21.0",
|
||||
"pydantic-settings>=2.0.0",
|
||||
"pyyaml>=6.0",
|
||||
"pytest>=8.4.1",
|
||||
]
|
||||
|
||||
[project.optional-dependencies]
|
||||
azure = [
|
||||
"azure-identity>=1.21.0",
|
||||
]
|
||||
providers = [
|
||||
"google-genai>=1.8.0",
|
||||
"anthropic>=0.49.0",
|
||||
|
|
|
|||
|
|
@ -29,30 +29,24 @@ class YamlSettingsSource(PydanticBaseSettingsSource):
|
|||
def replacer(match):
|
||||
var_name = match.group(1)
|
||||
default_value = match.group(3) if match.group(3) is not None else ''
|
||||
result = os.environ.get(var_name, default_value)
|
||||
|
||||
# Convert string booleans to actual booleans
|
||||
if result.lower() == 'true':
|
||||
return 'true' # Keep as string, let Pydantic handle conversion
|
||||
elif result.lower() == 'false':
|
||||
return 'false' # Keep as string, let Pydantic handle conversion
|
||||
return result
|
||||
return os.environ.get(var_name, default_value)
|
||||
|
||||
pattern = r'\$\{([^:}]+)(:([^}]*))?\}'
|
||||
|
||||
# Check if the entire value is a single env var expression with boolean default
|
||||
# Check if the entire value is a single env var expression
|
||||
full_match = re.fullmatch(pattern, value)
|
||||
if full_match:
|
||||
result = replacer(full_match)
|
||||
# If the result is a boolean string and the whole value was the env var,
|
||||
# return the actual boolean
|
||||
if result == 'true':
|
||||
return True
|
||||
elif result == 'false':
|
||||
return False
|
||||
# Convert boolean-like strings to actual booleans
|
||||
if isinstance(result, str):
|
||||
lower_result = result.lower().strip()
|
||||
if lower_result in ('true', '1', 'yes', 'on'):
|
||||
return True
|
||||
elif lower_result in ('false', '0', 'no', 'off', ''):
|
||||
return False
|
||||
return result
|
||||
else:
|
||||
# Otherwise, do string substitution
|
||||
# Otherwise, do string substitution (keep as strings for partial replacements)
|
||||
return re.sub(pattern, replacer, value)
|
||||
elif isinstance(value, dict):
|
||||
return {k: self._expand_env_vars(v) for k, v in value.items()}
|
||||
|
|
|
|||
|
|
@ -286,10 +286,10 @@ async def add_memory(
|
|||
)
|
||||
|
||||
# Adding structured JSON data
|
||||
# NOTE: episode_body must be a properly escaped JSON string. Note the triple backslashes
|
||||
# NOTE: episode_body should be a JSON string (standard JSON escaping)
|
||||
add_memory(
|
||||
name="Customer Profile",
|
||||
episode_body="{\\\"company\\\": {\\\"name\\\": \\\"Acme Technologies\\\"}, \\\"products\\\": [{\\\"id\\\": \\\"P001\\\", \\\"name\\\": \\\"CloudSync\\\"}, {\\\"id\\\": \\\"P002\\\", \\\"name\\\": \\\"DataMiner\\\"}]}",
|
||||
episode_body='{"company": {"name": "Acme Technologies"}, "products": [{"id": "P001", "name": "CloudSync"}, {"id": "P002", "name": "DataMiner"}]}',
|
||||
source="json",
|
||||
source_description="CRM data"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -2,11 +2,24 @@
|
|||
|
||||
from collections.abc import Callable
|
||||
|
||||
from azure.identity import DefaultAzureCredential, get_bearer_token_provider
|
||||
|
||||
|
||||
def create_azure_credential_token_provider() -> Callable[[], str]:
|
||||
"""Create Azure credential token provider for managed identity authentication."""
|
||||
"""
|
||||
Create Azure credential token provider for managed identity authentication.
|
||||
|
||||
Requires azure-identity package. Install with: pip install mcp-server[azure]
|
||||
|
||||
Raises:
|
||||
ImportError: If azure-identity package is not installed
|
||||
"""
|
||||
try:
|
||||
from azure.identity import DefaultAzureCredential, get_bearer_token_provider
|
||||
except ImportError:
|
||||
raise ImportError(
|
||||
'azure-identity is required for Azure AD authentication. '
|
||||
'Install it with: pip install mcp-server[azure]'
|
||||
) from None
|
||||
|
||||
credential = DefaultAzureCredential()
|
||||
token_provider = get_bearer_token_provider(
|
||||
credential, 'https://cognitiveservices.azure.com/.default'
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue