Rc/3.0 redux#49
Merged
jayckaiser merged 8 commits intomainfrom Dec 19, 2025
Merged
Conversation
* Refactor entire library to use EdFiSession from rc/0.3.0 branch; use logging library; remove Ed-Fi 2 functionality. * Fix bugs across repo. * Implements basic plain-text file cache for auth tokens * Make token base path configurable * Move token load/update from cache into separate method * Add test with forced refreshes * Refactor portalocker caching into its own class * Add pure Python soft lock based token cache * Have token caches implement abstract base class * For soft lockfile, impelement staleness threshold * Tweak tests and default timeout * Adds tests for get_token_info and for instance-specific auth endpoint * Add test for max_retries on paged requests * Add test for default behavior of retries off * Fix no-retry default test to add check for total call count * Make access_token a property to force authentication if accessed before first request. * Migrate get_instance_locator() code into instance_locator property. * Improve docstrings to clarify cleanup logic for posted records. * Update get_rows/pages docstrings and reorder arguments for clarity. * Update API year error to install latest 0.2 version. * Fix mock in get_token_info test to chdeck for JSON-encoded POST body * Incorporate from main auth URL fixes for specific API mode. * Feature/persist tokens to disk cleanup (#45) * Add Python 3.10 requirement (and fix indentation). * Clean up comments, whitespace, and method order in Session. * Move snapshot header setting outside internal auth helper method. * Pass max_retries argument into read_lock. * Move time attribute setters and cached token reauthentication into their respective helpers. * Clean up whitespacing, type hints, comments, and variable names; move shared definitions of `exists()` and `get_last_modified()` helpers into BaseTokenCache(). * Move shared abstract methods back into interface to allow future child class that caches non-locally. * Define refresh_at outside the helper call to ensure this argument can also be defined using cached tokens. * Bugfixes and make caller responsible for constructing TokenCache In preparation for removing PortalockerTokenCache (and leaving the pure Python LockfileTokenCache as the only option), shift instantiation and configuration of token caches onto the caller. - Replace `use_token_cache` parameter in EdFiClient with `token_cache`; caller must pass in instantiated token cache in order to use caching feature. - Remove `token_cache_lock_type` parameter and corresponding match/case from EdFiClient. - Add `token_id` property to BaseTokenCache abstract base class. Now that we expect a token cache to be instantiated outside of EdFiSession, sessions will have to pass in instance-client IDs, used to uniquely identify shared tokens, after instantiation. Implement such a setter in LockfileTokenCache that updates cache and lockfile paths upon being passed a new ID. - Update tests to follow this new pattern. Bug introduced in cleanup where EdFiSession.refresh_at was None even though EdFiSession.authenticated_at was not, occurring in non-cache code path. Session would call _make_auth_request, which does not update both timestamps. - Have _make_auth_request set both timestamps, not just authenticated_at. - Factor out token loading into EdFiSession._load_token_from_cache, a method that also updates both timestamps and shares the same signature as _make_auth_request. This should make the invariant more clear. Bug found in testing where LockfileTokenCache.get_write_lock would error out when `os.path.getmtime` was called on a nonexistent lockfile because the lockfile had been deleted in between the `os.path.exists` check and the `getmtime` call. Fixed with an extra branch in an existing try/except catching this FileNotFoundError. Could be addressed by another try/except that would allow us to try to acquire the write lock immediately, but this seems more readable and easier to reason about. * Remove portalocker caching implementation * Move parameters into __init__ to allow for caller to control token caches * Tweak test_multiprocessing_with_forced_refreshes interval to force more invalidations * Push uniqueness responsibility down to token caches instead of EdFiSession * Rename missing variable. * Rename object imported to match typing. * Linting/type checking fixes for token caching - Explicit `typing.cast` to narrow down types for Optional instance attributes - Fixing various timestamps to ints - Missing type hints - Default string/PathLike values to get rid of Optional types - Unused imports, unused variable fixes * Add mocked test for default uncached client fetching own token * Fix EdFiSession.connect type hint * Fix re-auth on stale token from cache * Update setup.py metadata for release. * Add additional comments for test against live ODS * Add token caching info to README --------- Co-authored-by: Alex Chen <achen@edanalytics.org> Co-authored-by: Alex C. <alchenist@users.noreply.github.com>
rlittle08
approved these changes
Dec 19, 2025
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.
No description provided.