-
-
Notifications
You must be signed in to change notification settings - Fork 495
fix: handle race condition in daily warning v0.x #2546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v0.x
Are you sure you want to change the base?
Changes from 4 commits
a4561ca
922d0d0
7efe039
4862abc
f959096
81f6bcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| # 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" | ||
|
|
||
| import logging | ||
| import os | ||
|
|
@@ -47,15 +47,24 @@ def _atomic_write_text(path: Path, text: str) -> None: | |
| last_date = None | ||
|
|
||
| if last_date != today: | ||
| warn( | ||
| "\nArviZ is undergoing a major refactor to improve flexibility and extensibility " | ||
| "while maintaining a user-friendly interface." | ||
| "\nSome upcoming changes may be backward incompatible." | ||
| "\nFor details and migration guidance, visit: " | ||
| "https://python.arviz.org/en/latest/user_guide/migration_guide.html", | ||
| FutureWarning, | ||
| ) | ||
| _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") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| with open(lock_file, "x", encoding="utf-8") as f: | ||
| f.write("1") | ||
|
player3zxcvbnm marked this conversation as resolved.
Outdated
|
||
| # If we get here, this process is the "winner" | ||
| warn( | ||
| "\nArviZ is undergoing a major refactor to improve flexibility and extensibility " | ||
| "while maintaining a user-friendly interface." | ||
| "\nSome upcoming changes may be backward incompatible." | ||
| "\nFor details and migration guidance, visit: " | ||
| "https://python.arviz.org/en/latest/user_guide/migration_guide.html", | ||
| FutureWarning, | ||
| ) | ||
| _atomic_write_text(stamp_file, today.isoformat()) | ||
| except FileExistsError: | ||
| # Another process already caught it and is warning/updating | ||
| pass | ||
|
|
||
|
|
||
| _warn_once_per_day() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo