Conversation
Introduce cache helpers for path/meta handling and atomic writes. Cover cache read/write and clearing flows with funcnodes pytest cases.
There was a problem hiding this comment.
Pull request overview
This PR bumps the version from 2.1.0 to 2.2.0 and introduces cache utility functions for managing cached data with atomic write operations and metadata support.
- Adds a new cache utilities module (
cache.py) with functions for managing cache directories, files, and metadata - Implements comprehensive test coverage for all cache utility functions
- Updates version across project files and adds changelog entry
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Version bump from 2.1.0 to 2.2.0 |
| uv.lock | Synchronized version update to 2.2.0 |
| CHANGELOG.md | Added v2.2.0 release entry documenting new cache utilities feature |
| src/funcnodes_core/utils/cache.py | New module providing cache directory management, atomic file writes, and metadata handling functions |
| tests/test_cache_utils.py | Comprehensive test suite covering all cache utility functions including edge cases and error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| def set_cache_meta_for(cache_path: Path, meta: dict[str, Any]): | ||
| """Write JSON metadata for a given cache file (atomic).""" |
There was a problem hiding this comment.
The docstring is missing documentation for the meta parameter. According to best practices, all function parameters should be documented, especially for public API functions.
| """Write JSON metadata for a given cache file (atomic).""" | |
| """ | |
| Write JSON metadata for a given cache file (atomic). | |
| Args: | |
| cache_path: Path to the cache file. | |
| meta: Metadata dictionary to write as JSON. | |
| """ |
|
|
||
|
|
||
| def get_cache_meta_for(cache_path: Path) -> Optional[dict[str, Any]]: | ||
| """Read JSON metadata for a given cache file, if present.""" |
There was a problem hiding this comment.
The docstring is missing documentation for the cache_path parameter. According to best practices, all function parameters should be documented, especially for public API functions.
| """Read JSON metadata for a given cache file, if present.""" | |
| """ | |
| Read JSON metadata for a given cache file, if present. | |
| Args: | |
| cache_path: Path to the cache file whose metadata should be read. | |
| """ |
| Write text to cache_path using a temp file + atomic replace. | ||
|
|
||
| This avoids partially-written cache files if the process crashes mid-write. |
There was a problem hiding this comment.
The docstring is missing documentation for parameters (cache_path, text, encoding) and lacks a description of the return value or behavior. According to best practices, all function parameters should be documented, especially for public API functions.
| Write text to cache_path using a temp file + atomic replace. | |
| This avoids partially-written cache files if the process crashes mid-write. | |
| Write text to a cache file atomically. | |
| This function writes the given text to the specified cache_path using a temporary file and atomic replace, | |
| which avoids partially-written cache files if the process crashes mid-write. | |
| Args: | |
| cache_path (Path): The path to the cache file to write. | |
| text (str): The text content to write to the cache file. | |
| encoding (str, optional): The encoding to use when writing the file. Defaults to "utf-8". | |
| Returns: | |
| None |
|
|
||
|
|
||
| @funcnodes_test | ||
| def test_chache_meta_exception_handling(): |
There was a problem hiding this comment.
The function name contains a typo: "chache" should be spelled "cache".
| def test_chache_meta_exception_handling(): | |
| def test_cache_meta_exception_handling(): |
| return None | ||
|
|
||
|
|
||
| def set_cache_meta_for(cache_path: Path, meta: dict[str, Any]): |
There was a problem hiding this comment.
The type annotation for the meta parameter is dict[str, Any], but the function actually accepts any type as demonstrated in the test where a string "hello" is passed. The type annotation should be Any to match the actual behavior, or the function should validate that only dict types are accepted.
| def set_cache_meta_for(cache_path: Path, meta: dict[str, Any]): | |
| def set_cache_meta_for(cache_path: Path, meta: Any): |
| return cache_path.with_suffix(cache_path.suffix + ".meta.json") | ||
|
|
||
|
|
||
| def get_cache_meta_for(cache_path: Path) -> Optional[dict[str, Any]]: |
There was a problem hiding this comment.
The return type annotation is Optional[dict[str, Any]], but the function can return any type that was stored via set_cache_meta_for. As shown in the test, a string "hello" can be stored and retrieved. The return type should be Optional[Any] to accurately reflect the actual behavior.
| def get_cache_meta_for(cache_path: Path) -> Optional[dict[str, Any]]: | |
| def get_cache_meta_for(cache_path: Path) -> Optional[Any]: |
No description provided.