-
Notifications
You must be signed in to change notification settings - Fork 164
feat(BA-3802): Migrate to SQLAlchemy 2.0 with comprehensive type safety improvements #7880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 18 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
d0ed749
deps: upgrade SQLAlchemy to 2.0.45
HyeockJinKim 48467d3
fix: SQLAlchemy 2.0 migration - Row indexing, select(), insert(), row…
HyeockJinKim f7d6ff2
fix: SQLAlchemy 2.0 migration - remaining Row indexing, select(), row…
HyeockJinKim 567ab9f
fix: SQLAlchemy 2.0 migration - TypeDecorator signatures
HyeockJinKim 3a2a0ec
fix: SQLAlchemy 2.0 migration - sessionmaker to async_sessionmaker
HyeockJinKim 449db2c
fix: SQLAlchemy 2.0 migration - ORM Row 2.0 style (batch 1)
HyeockJinKim 5c1d8d0
fix: SQLAlchemy 2.0 migration - ORM Row 2.0 style (batch 2)
HyeockJinKim 357349f
fix: SQLAlchemy 2.0 migration - ORM Row 2.0 style (batch 3)
HyeockJinKim 6122b85
fix: SQLAlchemy 2.0 migration - ORM Row 2.0 style (batch 4)
HyeockJinKim bd97f65
fix: SQLAlchemy 2.0 migration - ORM Row 2.0 style (batch 5)
HyeockJinKim bb73517
fix: SQLAlchemy 2.0 migration - ORM Row 2.0 style (batch 6)
HyeockJinKim fd8df4d
fix: SQLAlchemy 2.0 migration - ORM Row 2.0 style (batch 7)
HyeockJinKim 7468a58
fix: SQLAlchemy 2.0 migration - base.py type fixes
HyeockJinKim afb9432
fix: SQLAlchemy 2.0 migration - type fixes (batch 8)
HyeockJinKim 323f000
fix: SQLAlchemy 2.0 migration - repositories and Row type fixes
HyeockJinKim 1bca8c6
fix: SQLAlchemy 2.0 migration - QueryCondition/QueryOption and datacl…
HyeockJinKim 945b956
fix: SQLAlchemy 2.0 migration - type fixes in utils, repositories/bas…
HyeockJinKim 2708cae
fix: SQLAlchemy 2.0 migration - Row type fixes in models
HyeockJinKim 29308a3
fix: SQLAlchemy 2.0 migration - additional Row type fixes in models
HyeockJinKim 4f1d242
fix: Clean up VFolderRow type alias in api/vfolder.py
HyeockJinKim f4b9cc7
fix: SQLAlchemy 2.0 migration - type fixes in repositories and services
HyeockJinKim 93f83ae
fix: SQLAlchemy 2.0 migration - type fixes in artifact db_source and …
HyeockJinKim 93a5669
fix: SQLAlchemy 2.0 migration - additional type fixes in manager package
HyeockJinKim 98ecc3d
fix: SQLAlchemy 2.0 migration - fix type errors in resource_usage.py
HyeockJinKim a741987
fix: SQLAlchemy 2.0 migration - fix type errors in model_serving repo…
HyeockJinKim 8f6d5f9
fix: SQLAlchemy 2.0 migration - scheduler, repositories, event handlers
HyeockJinKim edb0d9c
fix: SQLAlchemy 2.0 migration - container_registry, group, domain repos
HyeockJinKim 2fe52ec
fix: SQLAlchemy 2.0 migration - repositories, scheduler, services
HyeockJinKim 5d29651
fix: SQLAlchemy 2.0 migration - gql_legacy and minilang type fixes
HyeockJinKim 38c9d3c
fix: SQLAlchemy 2.0 migration - appproxy/coordinator, account_manager…
HyeockJinKim 92cb5dd
Refactor code for improved readability and consistency
HyeockJinKim bb745d1
Add new fragment
HyeockJinKim c4d5ee4
fix: update SQLAlchemy relationships to use string-based join conditi…
HyeockJinKim 17b74d8
fix: SQLAlchemy 2.0 migration - Row access patterns and deprecated APIs
HyeockJinKim d3f0510
fix: remove unnecessary union type in AgentData.id
HyeockJinKim b4ab7cf
fix: update tests for SQLAlchemy 2.0 Row access patterns
HyeockJinKim ce0a810
chore: remove temporary task tracking file
HyeockJinKim 4f4472b
fix: update test_utils.py for SQLAlchemy 2.0 Row access patterns
HyeockJinKim c1edd80
fix: restore correct resource_policy format and fix type annotations
HyeockJinKim ee3ebf9
chore: update api schema dump
HyeockJinKim c52fb84
fix: refactor join conditions in EndpointRow, GroupRow, and VFolderRo…
HyeockJinKim 90207f7
fix: update KeyPairData dotfiles type to bytes and adjust related log…
HyeockJinKim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,163 @@ | ||
| # SQLAlchemy 2.0 Migration Task List | ||
|
|
||
| ## Current Status (2026-01-08) | ||
| - **Total Errors**: 1,374 errors in 175 files | ||
| - **Checked Files**: 3,138 source files | ||
|
|
||
| ## Completed Work | ||
|
|
||
| ### Phase 1: Basic Syntax Migration (Completed) | ||
| - [x] Row indexing: `row["column"]` → `row.column` (~360 errors) | ||
| - [x] Select list args: `select([col1, col2])` → `select(col1, col2)` (~260 errors) | ||
| - [x] Insert args: `insert(table, values)` → `insert(table).values(...)` (~10 errors) | ||
| - [x] Result.rowcount: Added `cast(CursorResult, result)` (~36 errors) | ||
|
|
||
| ### Phase 2: ORM Row 2.0 Migration (Completed - 6 batches) | ||
| - [x] Batch 1: 4 files (commit: 204e4f8ca) | ||
| - [x] Batch 2: endpoint/row.py (commit: 367ee0361) | ||
| - [x] Batch 3: session/row.py + kernel/row.py (commit: f19a2fcf7) | ||
| - [x] Batch 4: 12 files (commit: e2620ab38) | ||
| - [x] Batch 5: 15 files (commit: 21e605789) | ||
| - [x] Batch 6: IDColumn patterns (commit: 99cf04673) | ||
|
|
||
| ### Phase 3: Infrastructure (Completed) | ||
| - [x] TypeDecorator signature fixes | ||
| - [x] async_sessionmaker migration | ||
|
|
||
| --- | ||
|
|
||
| ## Remaining Work | ||
|
|
||
| ### High Priority - Core Files (by error count) | ||
|
|
||
| | File | Errors | Primary Issues | | ||
| |------|--------|----------------| | ||
| | src/ai/backend/manager/registry.py | 105 | `Mapping[str, Any]` attribute access, AgentId type | | ||
| | src/ai/backend/manager/api/vfolder.py | 71 | Row attribute access, type issues | | ||
| | src/ai/backend/manager/repositories/permission_controller/db_source/db_source.py | 66 | ORM query types | | ||
| | src/ai/backend/manager/models/session/row.py | 41 | Column vs runtime value types | | ||
| | src/ai/backend/manager/models/endpoint/row.py | 37 | Column vs runtime value types | | ||
| | src/ai/backend/manager/api/stream.py | 34 | Row attribute access | | ||
| | src/ai/backend/manager/repositories/schedule/repository.py | 29 | ORM query types | | ||
| | src/ai/backend/appproxy/coordinator/models/worker.py | 29 | ORM migration needed | | ||
| | src/ai/backend/appproxy/coordinator/models/circuit.py | 27 | ORM migration needed | | ||
| | src/ai/backend/manager/repositories/vfolder/repository.py | 26 | ORM query types | | ||
| | src/ai/backend/manager/models/user/row.py | 26 | Column vs runtime value types | | ||
|
|
||
| ### Error Type Categories | ||
|
|
||
| #### Category 1: Row/Mapping Attribute Access (High Priority) | ||
| **Total: ~60 errors** | ||
| ``` | ||
| "Mapping[str, Any]" has no attribute "id" (42) | ||
| "Mapping[str, Any]" has no attribute "host" (6) | ||
| etc. | ||
| ``` | ||
| **Fix**: Use proper Row class types instead of Mapping[str, Any] | ||
|
|
||
| #### Category 2: String Index on Rows (Medium Priority) | ||
| **Total: ~40 errors** | ||
| ``` | ||
| Invalid index type "str" for "str"; expected type "SupportsIndex | slice" | ||
| No overload variant of "__getitem__" of "Row" matches argument type "str" | ||
| ``` | ||
| **Fix**: Change `row["column"]` to `row.column` | ||
|
|
||
| #### Category 3: Boolean Expression Types (Medium Priority) | ||
| **Total: ~30 errors** | ||
| ``` | ||
| Argument 1 to "where" has incompatible type "bool" | ||
| Argument 2 to "and_" has incompatible type "bool" | ||
| Argument 2 to "join" has incompatible type "bool" | ||
| ``` | ||
| **Fix**: Use explicit comparison operators (e.g., `column == value` instead of `value`) | ||
|
|
||
| #### Category 4: select() List Args (Low Priority - Alembic) | ||
| **Total: ~20 errors** | ||
| ``` | ||
| No overload variant of "select" matches argument type "list[...]" | ||
| ``` | ||
| **Fix**: `select([cols])` → `select(*cols)` (mostly in alembic files - skip) | ||
|
|
||
| #### Category 5: Column vs Runtime Value Types (Medium Priority) | ||
| **Total: ~40 errors** | ||
| ``` | ||
| Incompatible types in assignment (expression has type "str", variable has type "Column[str]") | ||
| Argument "name" has incompatible type "Column[str]"; expected "str" | ||
| ``` | ||
| **Fix**: ORM 2.0 `Mapped[T]` type annotations fix these automatically | ||
|
|
||
| #### Category 6: Nullable Type Issues (Low Priority) | ||
| **Total: ~30 errors** | ||
| ``` | ||
| Item "None" of "Row[Any] | None" has no attribute "uuid" | ||
| Item "None" of "KernelRow | None" has no attribute "status" | ||
| ``` | ||
| **Fix**: Add proper None checks or use `cast()` where appropriate | ||
|
|
||
| #### Category 7: AppProxy Models (Separate Task) | ||
| **Total: ~100 errors** | ||
| Files: | ||
| - src/ai/backend/appproxy/coordinator/models/worker.py (29) | ||
| - src/ai/backend/appproxy/coordinator/models/circuit.py (27) | ||
| - src/ai/backend/appproxy/coordinator/api/worker_v2.py (20) | ||
| - src/ai/backend/appproxy/coordinator/api/health.py (14) | ||
| - etc. | ||
|
|
||
| **Fix**: Need ORM 2.0 migration for appproxy models | ||
|
|
||
| ### Alembic Files (Skip - Legacy Code) | ||
| ~200+ errors in alembic migration files | ||
| - `Inspector.from_engine(Connection)` type issues | ||
| - `existing_server_default` type issues | ||
| - `select([...])` list args | ||
| - `Row["column"]` indexing | ||
|
|
||
| **Decision**: These are legacy migration files, type errors don't affect runtime. | ||
|
|
||
| --- | ||
|
|
||
| ## Next Steps (Recommended Order) | ||
|
|
||
| ### Step 1: Fix remaining Row["column"] patterns | ||
| Files to check: | ||
| ```bash | ||
| grep -r 'row\["' src/ai/backend/manager/ --include="*.py" | grep -v alembic | grep -v __pycache__ | ||
| ``` | ||
|
|
||
| ### Step 2: Fix Boolean expression issues | ||
| Search pattern: | ||
| ```bash | ||
| grep -rE '\.where\((True|False|[a-z_]+)\)' src/ai/backend/manager/ --include="*.py" | ||
| ``` | ||
|
|
||
| ### Step 3: Fix Mapping[str, Any] attribute access | ||
| Primary file: `registry.py` (105 errors) | ||
| - Change function return types from `Mapping[str, Any]` to proper Row types | ||
|
|
||
| ### Step 4: AppProxy ORM Migration (Separate Task) | ||
| - appproxy/coordinator/models/* need same ORM 2.0 migration as manager | ||
|
|
||
| --- | ||
|
|
||
| ## Commands | ||
|
|
||
| ### Run full type check | ||
| ```bash | ||
| pants --no-colors --no-dynamic-ui check :: 2>&1 | tee /tmp/typecheck-results.txt | ||
| ``` | ||
|
|
||
| ### Count errors by file | ||
| ```bash | ||
| grep ": error:" /tmp/typecheck-results.txt | cut -d: -f1 | sort | uniq -c | sort -rn | head -30 | ||
| ``` | ||
|
|
||
| ### Count error types | ||
| ```bash | ||
| grep ": error:" /tmp/typecheck-results.txt | sed 's/.*: error: //' | sed 's/ \[.*//' | sort | uniq -c | sort -rn | head -30 | ||
| ``` | ||
|
|
||
| ### Check specific file errors | ||
| ```bash | ||
| grep "specific/file.py" /tmp/typecheck-results.txt | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make a new exception