refactor(services): migrate builtin_tools_manage_service to SQLAlchemy 2.0 select() API#34973
Conversation
…y 2.0 select() API
Pyrefly DiffNo changes detected. |
Pyrefly DiffNo changes detected. |
Pyrefly DiffNo changes detected. |
There was a problem hiding this comment.
Pull request overview
Migrates BuiltinToolManageService database access from legacy session.query() usage to SQLAlchemy 2.0 select() / session.scalar(s) APIs to improve typing and align with modern ORM patterns.
Changes:
- Replaced
session.query(...).first()/count()/update()/delete()patterns withselect(),session.scalar(s),session.execute(update/delete(...)), andfunc.count(...). - Updated boolean filtering to use
.is_(True)and introduced bulk update withsynchronize_session=False. - Adjusted unit tests to mock
session.scalar()/session.execute()instead ofsession.query()chains.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
api/services/tools/builtin_tools_manage_service.py |
Refactors ORM queries/updates/deletes to SQLAlchemy 2.0 select() style throughout the builtin tools management service. |
api/tests/unit_tests/services/tools/test_builtin_tools_manage_service.py |
Updates unit tests’ session mocks to match the new scalar() / execute()-based data access patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -274,7 +274,7 @@ def test_returns_user_client_params_when_exists( | |||
| mock_create_enc.return_value = (mock_encrypter, MagicMock()) | |||
|
|
|||
| user_client = MagicMock(oauth_params='{"encrypted": "data"}') | |||
There was a problem hiding this comment.
In this test, ToolOAuthTenantClient.oauth_params is a property that returns a dict parsed from encrypted_oauth_params (see api/models/tools.py), but the mock sets oauth_params to a JSON string. This makes the test less representative and could hide type/usage issues. Consider setting oauth_params to a dict (or mocking encrypted_oauth_params and oauth_params consistently with the model).
| user_client = MagicMock(oauth_params='{"encrypted": "data"}') | |
| user_client = MagicMock(oauth_params={"encrypted": "data"}) |
…y 2.0 select() API (langgenius#34973) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
part of #22668
Files:
api/services/tools/builtin_tools_manage_service.pyDescription:
Migrates 16 occurrences of legacy
session.query()in the builtin tool providermanagement service. Notable changes:
is_defaultclear viasession.execute(update(...).values(is_default=False).execution_options(synchronize_session=False))BuiltinToolProvider.is_default == True→.is_(True)session.query(...).count()→session.scalar(select(func.count(...)))delete,func,updateto SQLAlchemy imports