Skip to content

Cache models are module-global: a second TypeDAL rebinds them, and close() unbinds them for everyone #21

Closed

Description

@robinvandernoord

_TypedalCache and _TypedalCacheDependency (src/typedal/caching.py) are module-level TypedTable classes. Every TypeDAL(..., enable_typedal_caching=True) calls try_define on them in __init__, which binds the class to that database. The last-created DAL wins.

Consequences

  1. Cache rows can end up in another database. With two caching-enabled DALs in one process (e.g. an app DB and a reporting DB), save_to_cache, load_from_cache and remove_cache_for_table all go through _TypedalCache.where(...) / .insert(...), so they use whichever DAL was defined last, not the DAL the query ran on. The after-insert/update/delete invalidation hooks have the same problem.
  2. close() breaks caching for the other DALs. TypeDAL.close() calls unbind() on every model in its class map, including the shared cache models. Afterwards any insert on another DAL with caching fails in its after-insert hook:
    caching.py: remove_cache_for_table -> _TypedalCache.where(...)
    OSError: @define or db.define is not called on this class yet!
    
  3. The test suite depends on test order. On release/typedal-v6, running tests/test_async.py (its db_cached fixture creates and closes a caching DAL) or tests/test_config.py right before tests/test_relationships.py fails 26 tests:
    pytest tests/test_async.py tests/test_relationships.py   # 26 failed
    pytest tests/test_relationships.py                       # all pass
    
    It only passes in the full run because of alphabetical module order. test_upsert_invalidates_cache (fix: resolve open issues from the 6.0 release PR #19) works around it by restoring the previous binding with try_define afterwards.

Possible directions

  • Resolve the cache models per database: look them up through db._class_map / db.typedal_cache, or create per-instance subclasses in TypeDAL.__init__, and have the caching functions take the DAL that ran the query (instance.db, rows.db, the hook's Set.db).
  • At minimum, close() should not unbind models that are bound to a different DAL (model._db is not self).

Found while working on #19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions