refactor: migrate db.session.query to select in inner_api and web controllers#33774
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modernizes the database interaction layer by migrating existing SQLAlchemy 1.x query patterns to the more explicit and performant SQLAlchemy 2.0 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Pyrefly DiffNo changes detected. |
There was a problem hiding this comment.
Pull request overview
Migrates remaining legacy db.session.query(...).first() usage in selected inner API and web controllers to SQLAlchemy 2.0 select()-style queries to improve typing and align with the ongoing ORM modernization effort (#22668).
Changes:
- Replaced
db.session.query(...).where(...).first()withdb.session.scalar(select(...).where(...))across web/inner API controllers. - Updated inner-api plugin wrappers to use
Session.scalar(select(...))for end-user lookups. - Added
selectimports where needed.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| api/controllers/web/site.py | Migrates Site lookup to select() + scalar() in the site info endpoint. |
| api/controllers/web/human_input_form.py | Migrates App/Site lookups used when resolving web human-input forms. |
| api/controllers/inner_api/wraps.py | Migrates EndUser lookup in inner API auth wrapper. |
| api/controllers/inner_api/workspace/workspace.py | Migrates Account lookup by email for enterprise workspace creation. |
| api/controllers/inner_api/plugin/wraps.py | Migrates EndUser/Tenant lookups in plugin inner API wrappers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Code Review
This pull request migrates several database queries from the legacy db.session.query() to the modern SQLAlchemy 2.0 select() syntax. The changes are a good step towards modernizing the codebase. However, I've noted a few places where replacing .first() with db.session.scalar() changes the behavior of the query. db.session.scalar() is stricter and will raise an exception if multiple rows are returned, unlike .first(). While this can be a positive change by enforcing data integrity, it can also introduce new runtime errors if the queried columns are not guaranteed to be unique. I've left specific comments on the lines where this might be an issue.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
add |
Pyrefly DiffNo changes detected. |
1 similar comment
Pyrefly DiffNo changes detected. |
bbba844 to
b447c54
Compare
Pyrefly DiffNo changes detected. |
b447c54 to
9c87149
Compare
Pyrefly Diffbase → PR--- /tmp/pyrefly_base.txt 2026-03-19 17:26:01.285233192 +0000
+++ /tmp/pyrefly_pr.txt 2026-03-19 17:25:51.832284041 +0000
@@ -2076,29 +2076,29 @@
ERROR `SimpleNamespace` is not assignable to attribute `request` with type `Request` [bad-assignment]
--> tests/unit_tests/controllers/files/test_upload.py:170:26
ERROR Missing argument `tenant_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:168:44
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:171:44
ERROR Missing argument `user_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:168:44
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:171:44
ERROR Missing argument `tenant_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:185:31
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:188:31
ERROR Missing argument `user_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:185:31
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:188:31
ERROR Missing argument `tenant_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:200:35
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:203:35
ERROR Missing argument `user_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:200:35
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:203:35
ERROR Missing argument `tenant_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:222:44
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:225:44
ERROR Missing argument `user_model` in function `controllers.inner_api.plugin.wraps.decorated_view` [missing-argument]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:222:44
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:225:44
ERROR Argument `type[PluginTestPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:250:35
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:253:35
ERROR Argument `type[PluginTestPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:265:35
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:268:35
ERROR Argument `type[TestPluginData.test_should_raise_error_on_invalid_payload.InvalidPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:283:35
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:286:35
ERROR Argument `type[PluginTestPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
- --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:296:35
+ --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:299:35
ERROR `SimpleNamespace` is not assignable to attribute `db` with type `SQLAlchemy` [bad-assignment]
--> tests/unit_tests/controllers/mcp/test_mcp.py:19:17
ERROR `SimpleNamespace` is not assignable to attribute `mcp_ns` with type `Namespace` [bad-assignment]
|
|
Thank you for merging this. |
Summary
db.session.query()calls to SQLAlchemy 2.0select()API incontrollers/inner_api/andcontrollers/web/Changes
controllers/inner_api/wraps.py— 1 occurrencecontrollers/inner_api/plugin/wraps.py— 3 occurrences (including localSessionusage)controllers/inner_api/workspace/workspace.py— 1 occurrencecontrollers/web/human_input_form.py— 2 occurrencescontrollers/web/site.py— 1 occurrence