mirror of
https://github.com/encounter/adk-python.git
synced 2026-07-09 18:19:28 -07:00
fix(mcp): used logger to log instead of print
Merge https://github.com/google/adk-python/pull/4324 **Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: #4320 **2. Or, if no issue exists, describe the change:** **Problem:** Was using print instead of logger for warning message **Solution:* Refactored ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. _Please include a summary of passed `pytest` results._ **Manual End-to-End (E2E) Tests:** ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [ ] Any dependent changes have been merged and published in downstream modules. ### Additional context _Add any other context or screenshots about the feature request here._ COPYBARA_INTEGRATE_REVIEW=https://github.com/google/adk-python/pull/4324 from DineshThumma9:logger_in_mcp_session_clean 5eaa9bdc8c3c6697e75b0e876f043d496dd8ee95 PiperOrigin-RevId: 867601662
This commit is contained in:
committed by
Copybara-Service
parent
a2e43aaf19
commit
6bc70a6bab
@@ -434,10 +434,9 @@ class MCPSessionManager:
|
||||
await exit_stack.aclose()
|
||||
except Exception as e:
|
||||
# Log the error but don't re-raise to avoid blocking shutdown
|
||||
print(
|
||||
'Warning: Error during MCP session cleanup for'
|
||||
f' {session_key}: {e}',
|
||||
file=self._errlog,
|
||||
logger.warning(
|
||||
f'Error during MCP session cleanup for {session_key}',
|
||||
exc_info=True,
|
||||
)
|
||||
finally:
|
||||
del self._sessions[session_key]
|
||||
|
||||
@@ -388,7 +388,8 @@ class TestMCPSessionManager:
|
||||
assert len(manager._sessions) == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_close_with_errors(self):
|
||||
@patch("google.adk.tools.mcp_tool.mcp_session_manager.logger")
|
||||
async def test_close_with_errors(self, mock_logger):
|
||||
"""Test cleanup when some sessions fail to close."""
|
||||
manager = MCPSessionManager(self.mock_stdio_connection_params)
|
||||
|
||||
@@ -403,9 +404,6 @@ class TestMCPSessionManager:
|
||||
manager._sessions["session1"] = (session1, exit_stack1)
|
||||
manager._sessions["session2"] = (session2, exit_stack2)
|
||||
|
||||
custom_errlog = StringIO()
|
||||
manager._errlog = custom_errlog
|
||||
|
||||
# Should not raise exception
|
||||
await manager.close()
|
||||
|
||||
@@ -413,10 +411,11 @@ class TestMCPSessionManager:
|
||||
exit_stack2.aclose.assert_called_once()
|
||||
assert len(manager._sessions) == 0
|
||||
|
||||
# Error should be logged
|
||||
error_output = custom_errlog.getvalue()
|
||||
assert "Warning: Error during MCP session cleanup" in error_output
|
||||
assert "Close error 1" in error_output
|
||||
# Error should be logged via logger.warning
|
||||
mock_logger.warning.assert_called_once()
|
||||
args, kwargs = mock_logger.warning.call_args
|
||||
assert "Error during MCP session cleanup for session1" in args[0]
|
||||
assert kwargs.get("exc_info")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("google.adk.tools.mcp_tool.mcp_session_manager.stdio_client")
|
||||
|
||||
Reference in New Issue
Block a user