Conversation
- Replaced `peewee` with `tinydb` as the default storage backend in `pyproject.toml`. - Added support for both `tinydb` and `sqlite` in the evaluation row store and event bus database. - Refactored logger and database initialization to accommodate the new storage options. - Updated the SQLite-related classes to raise informative errors if `peewee` is not installed. - Ensured backward compatibility for existing SQLite logger and event bus implementations.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Changed default database filenames from "events.db" to "logs.db" and "events.json" to "logs.json" based on storage backend. - Added cache clearing and table reloading in TinyDBEventBusDatabase to ensure fresh reads of unprocessed events and improve cleanup accuracy.
dphuang2
changed the title
…nd event bus database - Introduced `TestCrossProcessCacheInvalidation` to verify that query caches are properly invalidated when another process modifies the database. - Added tests to ensure that both evaluation row stores and event bus databases can read fresh data and updates from other processes. - Implemented scenarios for both reading and writing operations across multiple simulated processes to validate cache behavior.
…reads in multi-process scenarios
…ate removal count and clear cache for fresh reads
- Introduced `test_delete_row_nonexistent` to verify that attempting to delete a non-existent row returns a count of 0 and confirms the store remains empty.
| # Temporarily restore the original function | ||
| dir_utils.find_eval_protocol_dir = _original_find_eval_protocol_dir | ||
| yield _original_find_eval_protocol_dir | ||
| # The session fixture will clean up when tests complete |
There was a problem hiding this comment.
Bug: Test fixture doesn't restore isolated directory function
The restore_original_find_eval_protocol_dir fixture sets dir_utils.find_eval_protocol_dir to the original function at line 69, but after yielding it doesn't restore it back to the isolated version. The comment claims "The session fixture will clean up when tests complete", but this only happens at session end. If test_directory_utils.py tests run mid-session before other tests, those subsequent tests will use the real ~/.eval_protocol directory instead of the isolated temp directory, potentially causing test pollution or corruption of the user's actual data.
| def delete_all_rows(self) -> int: | ||
| count = len(self._table) | ||
| self._table.truncate() | ||
| return count |
There was a problem hiding this comment.
Bug: Missing cache clear before counting rows in delete_all
The delete_all_rows method calls len(self._table) without first calling self._table.clear_cache(). In multi-process scenarios, the cached table data may be stale, causing the returned count to be inaccurate. Other methods like upsert_row, read_rows, and delete_row correctly call clear_cache() before reading from the table, but this method is inconsistent.
1 participant
peeweewithtinydbas the default storage backend inpyproject.toml.tinydbandsqlitein the evaluation row store and event bus database.peeweeis not installed.name: Pull Request
about: Propose changes to the codebase
title: "Brief description of changes"
labels: ''
assignees: ''
Description
Please include a summary of the change and which issue is fixed or feature is implemented. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Implements # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Test Configuration:
Checklist:
black .,isort .,flake8 .)Screenshots (if applicable)
If applicable, add screenshots to help showcase your changes.
Additional context
Add any other context about the PR here.
Note
Switches default storage to TinyDB and introduces pluggable backends for dataset logging and the cross-process event bus, with SQLite remaining via optional extra and full backward compatibility.
EvaluationRowStoreABC with implementations:TinyDBEvaluationRowStore(default) andSqliteEvaluationRowStore(optionalpeewee).get_evaluation_row_store(db_path)and_get_default_db_filename(); select backend viaEP_STORAGE(tinydbdefault).DatasetLoggerAdapterusing the store; keepSqliteDatasetLoggerAdapteras an alias; support no-op logger viaDISABLE_EP_SQLITE_LOG.CrossProcessEventBususing storage-backedEventBusDatabaseABC.TinyDBEventBusDatabaseandSqliteEventBusDatabase; addget_event_bus_database(db_path)and_get_default_db_filename(); keepSqliteEventBusas alias..eval_protocoldir to avoid TinyDB file contention.tinydb,tinyrecord; move SQLite to optional extrasqlite_storage.pyproject.tomland type-checker config (includetests); lockfile adjusted.Written by Cursor Bugbot for commit dbab765. This will update automatically on new commits. Configure here.