Skip to content

Fix pickle cache rewrite races#364

Merged
shaypal5 merged 7 commits intomasterfrom
codex/release-v4.3.0
Mar 13, 2026
Merged

Fix pickle cache rewrite races#364
shaypal5 merged 7 commits intomasterfrom
codex/release-v4.3.0

Conversation

@shaypal5
Copy link
Member

@shaypal5 shaypal5 commented Mar 6, 2026

Summary

  • make shared pickle-cache rewrites atomic with a temp file plus os.replace
  • use a sidecar lockfile outside cache directories for shared pickle-cache reads and writes so Windows can replace cache files safely
  • handle moved filesystem events in the pickle watchdog path
  • add regression coverage for concurrent shared-cache rewrites

Context

  • fixes the macOS cleanup failure in tests/test_cleanup.py::test_cleanup_stale_entries
  • fixes the Windows smoke-test regression from replacing an open cache file
  • preserves expected cache-directory contents for separate_files=True and separate_files=False

Files

  • src/cachier/cores/pickle.py
  • tests/pickle_tests/test_pickle_core.py

@shaypal5 shaypal5 added the codex label Mar 6, 2026
@shaypal5 shaypal5 requested a review from Copilot March 6, 2026 14:14
@shaypal5 shaypal5 requested a review from Borda March 6, 2026 14:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Bumps the Cachier package version to 4.3.0 for the v4.3.0 release.

Changes:

  • Updated the version string from 4.2.0 to 4.3.0

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

@shaypal5
Copy link
Member Author

shaypal5 commented Mar 7, 2026

@Borda please review (and ignore the tiny coverage gap - it seems to be a tiny bug, since 0 missing lines show up in the report).

Copy link
Contributor

@Borda Borda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just would think about grouping similar test to a class so the particular tests names could be shorter...

@shaypal5
Copy link
Member Author

LGTM, just would think about grouping similar test to a class so the particular tests names could be shorter...

Might be worth a chore issue, if you want to open one.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants