Skip to content

fix: handle race condition in daily warning v0.x#2546

Open
player3zxcvbnm wants to merge 6 commits intoarviz-devs:v0.xfrom
player3zxcvbnm:fix-daily-warning-v0x
Open

fix: handle race condition in daily warning v0.x#2546
player3zxcvbnm wants to merge 6 commits intoarviz-devs:v0.xfrom
player3zxcvbnm:fix-daily-warning-v0x

Conversation

@player3zxcvbnm
Copy link
Copy Markdown

Hi

I’ve implemented a fix for the race condition in _warn_once_per_day as discussed in #2530.

The Problem:
On Windows, parallel execution (e.g., pytest -n) causes a FileNotFoundError. This is due to multiple processes clashing over the temporary file creation and renaming process.

The Fix:

  • Replaced the "temporary file + rename" logic with an atomic lock-file pattern.
  • Uses open(path, "x") to ensure only one process creates the stamp file.
  • Catches FileExistsError so secondary processes skip the update rather than crashing.
  • Preserves the parents=True directory creation from Create parent directories for cache #2525.

Testing:
Verified on Windows 11. Parallel tests that previously failed now pass consistently.

Closes #2530

@player3zxcvbnm
Copy link
Copy Markdown
Author

Changes:

Encoding: Added encoding='utf-8' to file reads/writes.
Linting: Cleaned up unused imports and whitespace.

CI Failures:
They are all TypeErrors in the mcse diagnostics (specifically the sd method). These appear to be pre-existing environment issues.

Let me know if there's anything else I should adjust on my end.

Copy link
Copy Markdown
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

@Zethson or @schmoelder would either of you be able to test this PR? If it works I can make a patch release.

The version comment will need fixing before merging, but the other two can be ignored if the PR already fixes the issue.

Comment thread arviz/__init__.py Outdated
# pylint: disable=wildcard-import,invalid-name,wrong-import-position
"""ArviZ is a library for exploratory analysis of Bayesian models."""
__version__ = "0.23.4"
__version__ = "0.23.3"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

undo

Comment thread arviz/__init__.py
_atomic_write_text(stamp_file, today.isoformat())
try:
# Use a 'lock' file with today's date to ensure only one process warns
lock_file = stamp_file.with_suffix(f".{today}.lock")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure accumulating these lockfiles over time is a very good idea. We don't want the warning to be too overbearing but if it shows multiple times when importing arviz in parallel it would be perfectly fine too. If it works it might be fine though, this is a backport fix on a frozen version so no need to overthink it too much

Comment thread arviz/__init__.py Outdated
player3zxcvbnm and others added 2 commits March 5, 2026 02:16
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
@player3zxcvbnm
Copy link
Copy Markdown
Author

Applied the suggestion for the lockfile and bumped the version to 0.23.4. Ready for testing! My apologies for the repo confusion earlier—I'm still getting oriented with the ArviZ ecosystem.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants