Repository navigation
Fix multi-database caching isolation and remove global state - #22
Closed
robinvandernoord wants to merge 2 commits into
Closed
robinvandernoord wants to merge 2 commits into
robinvandernoord wants to merge 2 commits into
Conversation
`_TypedalCache` and `_TypedalCacheDependency` were defined directly on each caching-enabled TypeDAL, so the module-level classes stayed bound to the last database created: cache rows and invalidations from one database went to another, and `close()` on any database unbound them for all (#21). Each TypeDAL now defines per-instance subclasses (same table names), and the caching functions resolve them through `cache_models(db)` using the database the query, row or hook belongs to. `close()` also leaves models alone that are bound to a different database. BREAKING CHANGE: `clear_cache`, `clear_expired`, `remove_cache` and `remove_cache_for_table` take the database as first argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0181W827mjAmRu6ECjtQTU5v
Resolves the conflict in caching.py by keeping release/typedal-v6's invalidation logic (table-wide invalidation for plain Sets, skipping empty results) with the per-database `db` argument. Drops the module-global binding workaround from `test_upsert_invalidates_cache`, which per-database cache models make unnecessary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0181W827mjAmRu6ECjtQTU5v
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
This PR fixes a critical issue where multiple TypeDAL databases with caching enabled in the same process would share cache tables, causing data corruption and incorrect cache hits/misses. The fix introduces per-database cache model instances while maintaining consistent table names across databases.
Key Changes
Introduced
define_cache_models()andcache_models()functions: These functions manage per-database cache table bindings.define_cache_models()creates fresh subclasses of_TypedalCacheand_TypedalCacheDependencyfor each database to avoid binding conflicts, whilecache_models()retrieves the cache models bound to a specific database.Updated all cache operations to accept
dbparameter: Functions likeremove_cache(),remove_cache_for_table(),clear_cache(),clear_expired(),_insert_cache_entry(), and_fetch_cached_payload()now take aTypeDALinstance as their first parameter to ensure they operate on the correct database's cache tables.Removed global state dependency: Eliminated the
throw()helper and the pattern of relying on_TypedalCache._dbto determine which database to use. This was fragile and failed when multiple databases existed.Fixed
TypeDAL.close()unbinding logic: Modified to only unbind models that are actually bound to the closing database instance, preventing interference with other database instances that may share model classes.Updated cache invalidation hooks: Modified the after-insert, after-update, and before-delete hooks in
define.pyto capture the database instance and pass it to cache invalidation functions.Added comprehensive multi-database test: New test file
test_caching_multi_db.pyvalidates that two databases with caching enabled maintain separate caches and that closing one database doesn't break caching on another.Notable Implementation Details
typedal_cacheandtypedal_cache_dependencyacross all databases for consistency_TypedalCacheand_TypedalCacheDependencyclasses remain unbound and serve as base classes onlyRuntimeErrorif called on a database with caching disabledhttps://claude.ai/code/session_0181W827mjAmRu6ECjtQTU5v