fix(targets): reuse SQLAlchemy inspector reflection cache#3665
fix(targets): reuse SQLAlchemy inspector reflection cache#3665akashmalbari wants to merge 7 commits into
Conversation
Reviewer's GuideSQLConnector now caches and reuses a single SQLAlchemy Inspector instance across reflection methods and introduces a reflection cache clearing helper that is invoked after connector-driven DDL operations to prevent stale schema metadata; tests are added/updated to verify inspector reuse, cache clearing, and correct behavior after schema changes, plus a new AGENTS.md contributor guide is added. Sequence diagram for SQLConnector reflection reuse and cache invalidationsequenceDiagram
actor Client
participant SQLConnector
participant reflection_Inspector as Inspector
Client->>SQLConnector: table_exists(full_table_name)
SQLConnector->>SQLConnector: _inspector
SQLConnector->>Inspector: has_table(table_name, schema_name)
Inspector-->>SQLConnector: bool
SQLConnector-->>Client: bool
Client->>SQLConnector: get_table_columns(full_table_name)
SQLConnector->>SQLConnector: _inspector
SQLConnector->>Inspector: get_columns(table_name, schema_name)
Inspector-->>SQLConnector: columns
SQLConnector-->>Client: columns
Client->>SQLConnector: create_empty_table(full_table_name, columns, primary_key)
SQLConnector->>SQLConnector: create_empty_table(full_table_name, columns, primary_key)
SQLConnector-->>Client: None
Client->>SQLConnector: clear_reflection_cache()
SQLConnector->>SQLConnector: clear_reflection_cache()
alt Inspector has clear_cache
SQLConnector->>Inspector: clear_cache()
else uses info_cache
SQLConnector->>Inspector: info_cache.clear()
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Documentation build overview
27 files changed ·
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3665 +/- ##
==========================================
+ Coverage 94.13% 94.20% +0.06%
==========================================
Files 73 73
Lines 6208 6227 +19
Branches 763 767 +4
==========================================
+ Hits 5844 5866 +22
+ Misses 270 269 -1
+ Partials 94 92 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for the PR @akashmalbari! The problem of stale reflection makes me hesitant to accept this as-is. Developers can override many of the DDL methods here where you're calling I'll try to think about a different approach, but in the meantime wdyt? |
edgarrmondragon
left a comment
There was a problem hiding this comment.
See #3665 (comment).
|
Thanks for the review, @edgarrmondragon. I updated the approach so cache invalidation is no longer dependent on individual The connector now registers an engine-level I also added a regression test covering an overridden |
Closes #3478.
Summary
This updates
SQLConnectorto reuse a cached SQLAlchemyInspectorinstance for repeated reflection calls instead of creating a newsa.inspect(self._engine)for each call.The affected methods now share the connector-level Inspector:
table_exists()schema_exists()get_table_columns()This preserves SQLAlchemy's
Inspector.info_cacheacross reflection calls, allowing dialect-level reflection caching optimizations to work as intended.Cache invalidation
Because the Inspector now persists across calls, this also adds a reflection cache clearing helper and clears the cache after connector-managed DDL operations such as:
This avoids stale reflection after the SDK itself changes the target schema.
Tests
Added regression coverage for:
Validation
uv run pytest tests/sql/test_connector.py -quv run pytest tests/sql -quv sync --all-groups --all-extras --all-packagesuv run pytest tests -quvx nox -t typinguvx pre-commit run --all-filesSummary by Sourcery
Cache and reuse a SQLAlchemy Inspector within SQLConnector to enable reflection caching and invalidate it after connector-managed schema changes.
Bug Fixes:
Documentation:
Tests: