From eb39fd90b6317332fb53153b72f86c19ba64420d Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Tue, 26 May 2026 15:23:06 +0530 Subject: [PATCH 01/17] adding background update helper method --- packages/python/genai_prices/__init__.py | 17 ++++++- packages/python/genai_prices/update_prices.py | 48 +++++++++++++++++++ tests/test_update_prices.py | 19 ++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/packages/python/genai_prices/__init__.py b/packages/python/genai_prices/__init__.py index c9a7df1c..978bb3ff 100644 --- a/packages/python/genai_prices/__init__.py +++ b/packages/python/genai_prices/__init__.py @@ -6,10 +6,23 @@ from . import data_snapshot, types from .types import Usage -from .update_prices import UpdatePrices, wait_prices_updated_async, wait_prices_updated_sync +from .update_prices import ( + UpdatePrices, + update_prices_in_background, + wait_prices_updated_async, + wait_prices_updated_sync, +) __version__ = _metadata_version('genai_prices') -__all__ = 'Usage', 'calc_price', 'UpdatePrices', 'wait_prices_updated_sync', 'wait_prices_updated_async', '__version__' +__all__ = ( + 'Usage', + 'calc_price', + 'UpdatePrices', + 'wait_prices_updated_sync', + 'wait_prices_updated_async', + 'update_prices_in_background', + '__version__', +) @overload diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index a6dd6fe4..491c20d9 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -20,6 +20,8 @@ logger = logging.getLogger('genai-prices') DEFAULT_UPDATE_URL = 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json' _global_update_prices: UpdatePrices | None = None +_global_update_prices_lock = threading.Lock() +_global_update_prices_ref_count: int = 0 def wait_prices_updated_sync(timeout: float | None = None) -> bool: @@ -48,6 +50,47 @@ async def wait_prices_updated_async(timeout: float | None = None) -> bool: return await asyncio.to_thread(wait_prices_updated_sync, timeout) +def update_prices_in_background() -> UpdatePricesHandle: + """Update prices in the background using a daemon thread. + + Args: + update_interval: The interval to update prices in seconds. + """ + + global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices + + with _global_update_prices_lock: + if _global_update_prices_ref_count == 0: + # Should I touch the global state here or let it be throwaway variable anyway? + # I wonder if I could trigger without holding the variable so that it looks more throwaway? + update_prices = UpdatePrices() + update_prices.start() + _global_update_prices_ref_count += 1 + + return UpdatePricesHandle() + + +@dataclass +class UpdatePricesHandle: + """A handle for the update prices in the background.""" + + _closed: bool = field(default=False, init=False) + + def close(self): + global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices + if _global_update_prices is None: + # Maybe consider raising an error here? + return + with _global_update_prices_lock: + if self._closed: + return + _global_update_prices_ref_count -= 1 + if _global_update_prices_ref_count == 0: + _global_update_prices.stop() + self._closed = True + return + + @dataclass class UpdatePrices: """Update prices in the background using a daemon thread. @@ -78,10 +121,15 @@ def start(self, *, wait: bool | float = False): if self._thread is not None: raise RuntimeError('UpdatePrices background task already started') + # Maybe we need to change up a few things right here? + # We cannot keep raising here because we do need multiple threads to be able to at least call it + # Whether we start the updater is a different thing + if _global_update_prices is not None: raise RuntimeError( 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' ) + return _global_update_prices = self self._prices_updated.clear() diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 71bd16e9..271e6c99 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -9,6 +9,7 @@ Usage, calc_price, data_snapshot, + update_prices_in_background, wait_prices_updated_async, wait_prices_updated_sync, ) @@ -95,3 +96,21 @@ def test_update_prices_multiple(): ): with UpdatePrices(): pass + + +def test_update_prices_ref_count(): + # Both should get the same global updater from genai-prices + handle_1 = update_prices_in_background() + handle_2 = update_prices_in_background() + assert handle_1 is not handle_2 + handle_1.close() + handle_2.close() + assert data_snapshot._custom_snapshot is not None + + +def test_update_prices_manual_with_global(): + handle = update_prices_in_background() + with UpdatePrices(): + assert data_snapshot._custom_snapshot is not None + handle.close() + assert data_snapshot._custom_snapshot is not None From cb1e7b76436279c708749e4225e5ddc8c45f8be1 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Tue, 26 May 2026 15:33:49 +0530 Subject: [PATCH 02/17] fix in structure, do not rely on ref count to see if updater is running --- packages/python/genai_prices/update_prices.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 491c20d9..01e947c6 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -60,11 +60,8 @@ def update_prices_in_background() -> UpdatePricesHandle: global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices with _global_update_prices_lock: - if _global_update_prices_ref_count == 0: - # Should I touch the global state here or let it be throwaway variable anyway? - # I wonder if I could trigger without holding the variable so that it looks more throwaway? - update_prices = UpdatePrices() - update_prices.start() + if _global_update_prices is None: + _throwaway = UpdatePrices().start() _global_update_prices_ref_count += 1 return UpdatePricesHandle() @@ -78,17 +75,15 @@ class UpdatePricesHandle: def close(self): global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices - if _global_update_prices is None: + if _global_update_prices is None or self._closed: # Maybe consider raising an error here? return + with _global_update_prices_lock: - if self._closed: - return _global_update_prices_ref_count -= 1 if _global_update_prices_ref_count == 0: _global_update_prices.stop() self._closed = True - return @dataclass From f28bb53ece20e786d77902f33f38a239f67bcebd Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 11:27:31 +0530 Subject: [PATCH 03/17] Make the shared background updater safe for library consumers update_prices_in_background() is intended for logfire and pydantic-ai to opt into price updates independently without duplicating updater threads. Harden the handle/refcount design for that use: - Bind each UpdatePricesHandle to the updater it was created for, so directly-constructed or stale handles are inert and cannot stop an updater they never claimed - Return an inert handle instead of raising when a manually started UpdatePrices is already keeping prices fresh - Never raise from UpdatePricesHandle.close(); log background fetch errors instead so consumers can close in shutdown paths - Add GENAI_PRICES_DISABLE_AUTO_UPDATE env var as a process-wide opt-out - Document shared-updater semantics: revert to bundled data on last close, non-blocking first fetch, and the inert-handle caveat Co-Authored-By: Claude Fable 5 --- packages/python/README.md | 36 +++++ packages/python/genai_prices/__init__.py | 2 + packages/python/genai_prices/update_prices.py | 153 ++++++++++++++---- tests/test_update_prices.py | 109 +++++++++++-- 4 files changed, 255 insertions(+), 45 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index 9fdd7f19..1769fa3c 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -126,6 +126,42 @@ update_prices.stop() # stop updating prices Only one `UpdatePrices` instance can be running at a time. +For libraries and integrations that want to opt into updating prices without creating duplicate background +threads, use `update_prices_in_background()`: + +```py +from genai_prices import update_prices_in_background + +update_prices_handle = update_prices_in_background() +... +update_prices_handle.close() +``` + +The first call starts a shared process-wide updater with default settings (hourly refresh from the default URL — +the shared updater is not configurable; if you need a custom URL or interval, use `UpdatePrices` directly). Later +calls reuse the same updater and return independent handles. The updater is stopped when the last handle is +closed, at which point prices revert to the data bundled with the installed package. + +If an `UpdatePrices` instance has already been started manually, `update_prices_in_background()` does not start a +second updater and returns a handle that does nothing on close: prices are already being kept up to date, and the +manual updater's lifetime stays with whoever started it. Note that such a handle stays inert: if the manual +updater is later stopped, background updates stop with it — call `update_prices_in_background()` again to start a +new shared updater. + +`UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged instead. + +`update_prices_in_background()` returns immediately and never blocks on the download. Until the first fetch +completes, `calc_price` keeps using the data bundled with the installed package, so prices for models released +after that snapshot may be missing for the first moments of the process. Once the fetch lands, every subsequent +calculation uses the fresh data — prices computed before then are not recalculated. If you need fresh prices +before calculating (e.g. in a short-lived script), call `wait_prices_updated_sync()` / +`wait_prices_updated_async()` after acquiring the handle. + +To disable background updates entirely (e.g. in air-gapped environments, or when a library enables them on your +behalf), set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value: +`update_prices_in_background()` then returns a do-nothing handle and makes no network requests. This does not +affect manually created `UpdatePrices` instances. + If you'd like to wait for prices to be updated without access to the `UpdatePrices` instance, you can use the `wait_prices_updated_sync` function: ```py diff --git a/packages/python/genai_prices/__init__.py b/packages/python/genai_prices/__init__.py index 978bb3ff..2764f994 100644 --- a/packages/python/genai_prices/__init__.py +++ b/packages/python/genai_prices/__init__.py @@ -8,6 +8,7 @@ from .types import Usage from .update_prices import ( UpdatePrices, + UpdatePricesHandle, update_prices_in_background, wait_prices_updated_async, wait_prices_updated_sync, @@ -21,6 +22,7 @@ 'wait_prices_updated_sync', 'wait_prices_updated_async', 'update_prices_in_background', + 'UpdatePricesHandle', '__version__', ) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index f6cd177f..abaa1d87 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -2,6 +2,7 @@ import asyncio import logging +import os import threading from dataclasses import dataclass, field from time import time @@ -13,6 +14,8 @@ __all__ = ( 'DEFAULT_UPDATE_URL', 'UpdatePrices', + 'UpdatePricesHandle', + 'update_prices_in_background', 'wait_prices_updated_sync', 'wait_prices_updated_async', ) @@ -20,8 +23,9 @@ logger = logging.getLogger('genai-prices') DEFAULT_UPDATE_URL = 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json' _global_update_prices: UpdatePrices | None = None -_global_update_prices_lock = threading.Lock() -_global_update_prices_ref_count: int = 0 +_managed_update_prices: UpdatePrices | None = None +_managed_update_prices_ref_count = 0 +_global_update_prices_lock = threading.RLock() def wait_prices_updated_sync(timeout: float | None = None) -> bool: @@ -33,8 +37,11 @@ def wait_prices_updated_sync(timeout: float | None = None) -> bool: Returns: True if prices were updated, False otherwise. """ - if _global_update_prices: - return _global_update_prices.wait(timeout) + with _global_update_prices_lock: + update_prices = _global_update_prices + + if update_prices is not None: + return update_prices.wait(timeout) return False @@ -51,39 +58,97 @@ async def wait_prices_updated_async(timeout: float | None = None) -> bool: def update_prices_in_background() -> UpdatePricesHandle: - """Update prices in the background using a daemon thread. + """Update prices in the background using a shared, process-wide daemon thread. - Args: - update_interval: The interval to update prices in seconds. + The first call starts the shared updater; later calls reuse it and return independent handles. + The updater is stopped when the last handle is closed, at which point prices revert to the + data bundled with the installed package. + + This function returns immediately and never blocks on the download: until the first fetch + completes, price calculations keep using the bundled data, and prices computed before then + are not recalculated once fresh data lands. Use `wait_prices_updated_sync` or + `wait_prices_updated_async` if you need fresh prices before calculating. + + If an `UpdatePrices` instance has already been started manually, no second updater is started + and the returned handle does nothing on close — prices are already being kept up to date. Such + a handle stays inert: if the manual updater is later stopped, background updates stop with it, + and a new handle must be acquired to restart them. + + Set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value to + disable this function entirely: it returns a do-nothing handle and no updater is started. + + Returns: + A handle that stops the shared background updater when all handles have been closed. """ + global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count - global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices + if os.environ.get('GENAI_PRICES_DISABLE_AUTO_UPDATE'): + return UpdatePricesHandle() with _global_update_prices_lock: - if _global_update_prices is None: - _throwaway = UpdatePrices().start() - _global_update_prices_ref_count += 1 + if _managed_update_prices is not None: + _managed_update_prices_ref_count += 1 + return UpdatePricesHandle(_managed_update_prices) - return UpdatePricesHandle() + if _global_update_prices is not None: + # A manually started UpdatePrices is already keeping prices fresh; don't start a + # second updater and don't claim the manual one — its owner controls its lifetime. + return UpdatePricesHandle() + + update_prices = UpdatePrices() + _global_update_prices = update_prices + _managed_update_prices = update_prices + _managed_update_prices_ref_count = 1 + try: + update_prices._start_thread() # pyright: ignore[reportPrivateUsage] + except Exception: + _global_update_prices = None + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + raise + + return UpdatePricesHandle(update_prices) @dataclass class UpdatePricesHandle: - """A handle for the update prices in the background.""" + """A claim on the shared background updater started by `update_prices_in_background`. + A handle only releases the updater it was created for: handles constructed directly, or + outliving the updater they belong to, are inert and closing them does nothing. + """ + + _update_prices: UpdatePrices | None = None _closed: bool = field(default=False, init=False) def close(self): - global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices - if _global_update_prices is None or self._closed: - # Maybe consider raising an error here? - return + """Release this handle's claim on the shared updater. + + Stops the updater if this was the last open handle. Idempotent, and never raises: + errors from the background updater are logged instead. + """ + global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count with _global_update_prices_lock: - _global_update_prices_ref_count -= 1 - if _global_update_prices_ref_count == 0: - _global_update_prices.stop() + if self._closed: + return + self._closed = True + if self._update_prices is None or _managed_update_prices is not self._update_prices: + return + + _managed_update_prices_ref_count -= 1 + if _managed_update_prices_ref_count > 0: + return + + update_prices = self._update_prices + _global_update_prices = None + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + try: + update_prices._stop_thread() # pyright: ignore[reportPrivateUsage] + except Exception as e: + logger.error('Error from genai-prices background updater while closing (%s): %s', type(e).__name__, e) @dataclass @@ -113,27 +178,34 @@ def start(self, *, wait: bool | float = False): """ global _global_update_prices - if self._thread is not None: - raise RuntimeError('UpdatePrices background task already started') + with _global_update_prices_lock: + if self._thread is not None: + raise RuntimeError('UpdatePrices background task already started') - # Maybe we need to change up a few things right here? - # We cannot keep raising here because we do need multiple threads to be able to at least call it - # Whether we start the updater is a different thing + if _global_update_prices is not None: + raise RuntimeError( + 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' + ) - if _global_update_prices is not None: - raise RuntimeError( - 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' - ) - return + _global_update_prices = self + try: + self._start_thread() + except Exception: + _global_update_prices = None + raise + + if wait: + self.wait(timeout=30 if wait is True else wait) + + def _start_thread(self) -> None: + if self._thread is not None: + raise RuntimeError('UpdatePrices background task already started') - _global_update_prices = self self._prices_updated.clear() self._stop_event.clear() self._background_exc = None self._thread = threading.Thread(target=self._background_task, daemon=True, name='genai_prices:update') self._thread.start() - if wait: - self.wait(timeout=30 if wait is True else wait) def wait(self, timeout: float | None = None) -> bool: """Wait for the prices to be updated in the background task. @@ -150,9 +222,20 @@ def wait(self, timeout: float | None = None) -> bool: def stop(self): """Stop the background task.""" - global _global_update_prices + global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count + + with _global_update_prices_lock: + if self._thread is None: + return + + if _global_update_prices is self: + _global_update_prices = None + if _managed_update_prices is self: + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + self._stop_thread() - _global_update_prices = None + def _stop_thread(self) -> None: if self._thread is not None: self._stop_event.set() self._thread.join() diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 47df1111..5fc83e4e 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -7,13 +7,16 @@ from genai_prices import ( UpdatePrices, + UpdatePricesHandle, Usage, calc_price, data_snapshot, + update_prices as update_prices_module, update_prices_in_background, wait_prices_updated_async, wait_prices_updated_sync, ) +from genai_prices.update_prices import DEFAULT_UPDATE_URL pytestmark = pytest.mark.anyio @@ -35,7 +38,7 @@ def raise_for_status(self) -> None: def fake_get(url: str, timeout: httpx2.Timeout) -> Response: assert url in { 'https://example.test/prices.json', - 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json', + DEFAULT_UPDATE_URL, } assert timeout is not None return Response(content) @@ -181,19 +184,105 @@ def test_update_prices_multiple(monkeypatch: pytest.MonkeyPatch): pass -def test_update_prices_ref_count(): - # Both should get the same global updater from genai-prices +def test_update_prices_in_background_inert_when_manual_updater_running(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + with UpdatePrices(): + handle = update_prices_in_background() + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + + handle.close() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None + + +def test_update_prices_in_background_ref_count(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) handle_1 = update_prices_in_background() handle_2 = update_prices_in_background() - assert handle_1 is not handle_2 - handle_1.close() - handle_2.close() - assert data_snapshot._custom_snapshot is not None + try: + assert handle_1 is not handle_2 + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + handle_1.close() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None + + handle_1.close() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None -def test_update_prices_manual_with_global(): + handle_2.close() + assert data_snapshot._custom_snapshot is None + assert not wait_prices_updated_sync(timeout=0) + finally: + handle_1.close() + handle_2.close() + data_snapshot.set_custom_snapshot(None) + + +def test_update_prices_in_background_conflicts_with_manual_start(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) handle = update_prices_in_background() - with UpdatePrices(): + try: + with pytest.raises( + RuntimeError, + match='UpdatePrices global task already started, only one UpdatePrices can be active at a time', + ): + with UpdatePrices(): + pass + finally: + handle.close() + data_snapshot.set_custom_snapshot(None) + + +def test_update_prices_handle_directly_constructed_is_inert(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + handle = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + + UpdatePricesHandle().close() + assert wait_prices_updated_sync(timeout=0) assert data_snapshot._custom_snapshot is not None + finally: + handle.close() + data_snapshot.set_custom_snapshot(None) + assert data_snapshot._custom_snapshot is None + + +def test_update_prices_handle_close_does_not_raise_after_failed_fetch( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +): + def fake_get(*_args: object, **_kwargs: object) -> object: + raise httpx2.ConnectError('network down') + + monkeypatch.setattr(httpx2, 'get', fake_get) + handle = update_prices_in_background() + try: + # Wait on the internal event rather than wait_prices_updated_sync, which would + # consume the stored exception that close() is expected to log instead of raise. + updater = update_prices_module._managed_update_prices + assert updater is not None + assert updater._prices_updated.wait(timeout=5) + finally: + with caplog.at_level('ERROR', logger='genai-prices'): + handle.close() + data_snapshot.set_custom_snapshot(None) + assert any('while closing' in record.message for record in caplog.records) + + +def test_update_prices_in_background_disabled_by_env_var(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('GENAI_PRICES_DISABLE_AUTO_UPDATE', '1') + + def fail_get(*_args: object, **_kwargs: object) -> object: + raise AssertionError('no network requests should be made') + + monkeypatch.setattr(httpx2, 'get', fail_get) + handle = update_prices_in_background() + assert not wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is None handle.close() - assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None From 6046622c6db7017e0062c1ffe4480535fbd00f45 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 11:27:31 +0530 Subject: [PATCH 04/17] Make the shared background updater safe for library consumers update_prices_in_background() is intended for logfire and pydantic-ai to opt into price updates independently without duplicating updater threads. Harden the handle/refcount design for that use: - Bind each UpdatePricesHandle to the updater it was created for, so directly-constructed or stale handles are inert and cannot stop an updater they never claimed - Return an inert handle instead of raising when a manually started UpdatePrices is already keeping prices fresh - Never raise from UpdatePricesHandle.close(); log background fetch errors instead so consumers can close in shutdown paths - Add GENAI_PRICES_DISABLE_AUTO_UPDATE env var as a process-wide opt-out - Document shared-updater semantics: revert to bundled data on last close, non-blocking first fetch, and the inert-handle caveat --- packages/python/README.md | 36 +++++ packages/python/genai_prices/__init__.py | 2 + packages/python/genai_prices/update_prices.py | 153 ++++++++++++++---- tests/test_update_prices.py | 109 +++++++++++-- 4 files changed, 255 insertions(+), 45 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index 9fdd7f19..1769fa3c 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -126,6 +126,42 @@ update_prices.stop() # stop updating prices Only one `UpdatePrices` instance can be running at a time. +For libraries and integrations that want to opt into updating prices without creating duplicate background +threads, use `update_prices_in_background()`: + +```py +from genai_prices import update_prices_in_background + +update_prices_handle = update_prices_in_background() +... +update_prices_handle.close() +``` + +The first call starts a shared process-wide updater with default settings (hourly refresh from the default URL — +the shared updater is not configurable; if you need a custom URL or interval, use `UpdatePrices` directly). Later +calls reuse the same updater and return independent handles. The updater is stopped when the last handle is +closed, at which point prices revert to the data bundled with the installed package. + +If an `UpdatePrices` instance has already been started manually, `update_prices_in_background()` does not start a +second updater and returns a handle that does nothing on close: prices are already being kept up to date, and the +manual updater's lifetime stays with whoever started it. Note that such a handle stays inert: if the manual +updater is later stopped, background updates stop with it — call `update_prices_in_background()` again to start a +new shared updater. + +`UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged instead. + +`update_prices_in_background()` returns immediately and never blocks on the download. Until the first fetch +completes, `calc_price` keeps using the data bundled with the installed package, so prices for models released +after that snapshot may be missing for the first moments of the process. Once the fetch lands, every subsequent +calculation uses the fresh data — prices computed before then are not recalculated. If you need fresh prices +before calculating (e.g. in a short-lived script), call `wait_prices_updated_sync()` / +`wait_prices_updated_async()` after acquiring the handle. + +To disable background updates entirely (e.g. in air-gapped environments, or when a library enables them on your +behalf), set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value: +`update_prices_in_background()` then returns a do-nothing handle and makes no network requests. This does not +affect manually created `UpdatePrices` instances. + If you'd like to wait for prices to be updated without access to the `UpdatePrices` instance, you can use the `wait_prices_updated_sync` function: ```py diff --git a/packages/python/genai_prices/__init__.py b/packages/python/genai_prices/__init__.py index 978bb3ff..2764f994 100644 --- a/packages/python/genai_prices/__init__.py +++ b/packages/python/genai_prices/__init__.py @@ -8,6 +8,7 @@ from .types import Usage from .update_prices import ( UpdatePrices, + UpdatePricesHandle, update_prices_in_background, wait_prices_updated_async, wait_prices_updated_sync, @@ -21,6 +22,7 @@ 'wait_prices_updated_sync', 'wait_prices_updated_async', 'update_prices_in_background', + 'UpdatePricesHandle', '__version__', ) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index f6cd177f..abaa1d87 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -2,6 +2,7 @@ import asyncio import logging +import os import threading from dataclasses import dataclass, field from time import time @@ -13,6 +14,8 @@ __all__ = ( 'DEFAULT_UPDATE_URL', 'UpdatePrices', + 'UpdatePricesHandle', + 'update_prices_in_background', 'wait_prices_updated_sync', 'wait_prices_updated_async', ) @@ -20,8 +23,9 @@ logger = logging.getLogger('genai-prices') DEFAULT_UPDATE_URL = 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json' _global_update_prices: UpdatePrices | None = None -_global_update_prices_lock = threading.Lock() -_global_update_prices_ref_count: int = 0 +_managed_update_prices: UpdatePrices | None = None +_managed_update_prices_ref_count = 0 +_global_update_prices_lock = threading.RLock() def wait_prices_updated_sync(timeout: float | None = None) -> bool: @@ -33,8 +37,11 @@ def wait_prices_updated_sync(timeout: float | None = None) -> bool: Returns: True if prices were updated, False otherwise. """ - if _global_update_prices: - return _global_update_prices.wait(timeout) + with _global_update_prices_lock: + update_prices = _global_update_prices + + if update_prices is not None: + return update_prices.wait(timeout) return False @@ -51,39 +58,97 @@ async def wait_prices_updated_async(timeout: float | None = None) -> bool: def update_prices_in_background() -> UpdatePricesHandle: - """Update prices in the background using a daemon thread. + """Update prices in the background using a shared, process-wide daemon thread. - Args: - update_interval: The interval to update prices in seconds. + The first call starts the shared updater; later calls reuse it and return independent handles. + The updater is stopped when the last handle is closed, at which point prices revert to the + data bundled with the installed package. + + This function returns immediately and never blocks on the download: until the first fetch + completes, price calculations keep using the bundled data, and prices computed before then + are not recalculated once fresh data lands. Use `wait_prices_updated_sync` or + `wait_prices_updated_async` if you need fresh prices before calculating. + + If an `UpdatePrices` instance has already been started manually, no second updater is started + and the returned handle does nothing on close — prices are already being kept up to date. Such + a handle stays inert: if the manual updater is later stopped, background updates stop with it, + and a new handle must be acquired to restart them. + + Set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value to + disable this function entirely: it returns a do-nothing handle and no updater is started. + + Returns: + A handle that stops the shared background updater when all handles have been closed. """ + global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count - global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices + if os.environ.get('GENAI_PRICES_DISABLE_AUTO_UPDATE'): + return UpdatePricesHandle() with _global_update_prices_lock: - if _global_update_prices is None: - _throwaway = UpdatePrices().start() - _global_update_prices_ref_count += 1 + if _managed_update_prices is not None: + _managed_update_prices_ref_count += 1 + return UpdatePricesHandle(_managed_update_prices) - return UpdatePricesHandle() + if _global_update_prices is not None: + # A manually started UpdatePrices is already keeping prices fresh; don't start a + # second updater and don't claim the manual one — its owner controls its lifetime. + return UpdatePricesHandle() + + update_prices = UpdatePrices() + _global_update_prices = update_prices + _managed_update_prices = update_prices + _managed_update_prices_ref_count = 1 + try: + update_prices._start_thread() # pyright: ignore[reportPrivateUsage] + except Exception: + _global_update_prices = None + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + raise + + return UpdatePricesHandle(update_prices) @dataclass class UpdatePricesHandle: - """A handle for the update prices in the background.""" + """A claim on the shared background updater started by `update_prices_in_background`. + A handle only releases the updater it was created for: handles constructed directly, or + outliving the updater they belong to, are inert and closing them does nothing. + """ + + _update_prices: UpdatePrices | None = None _closed: bool = field(default=False, init=False) def close(self): - global _global_update_prices_ref_count, _global_update_prices_lock, _global_update_prices - if _global_update_prices is None or self._closed: - # Maybe consider raising an error here? - return + """Release this handle's claim on the shared updater. + + Stops the updater if this was the last open handle. Idempotent, and never raises: + errors from the background updater are logged instead. + """ + global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count with _global_update_prices_lock: - _global_update_prices_ref_count -= 1 - if _global_update_prices_ref_count == 0: - _global_update_prices.stop() + if self._closed: + return + self._closed = True + if self._update_prices is None or _managed_update_prices is not self._update_prices: + return + + _managed_update_prices_ref_count -= 1 + if _managed_update_prices_ref_count > 0: + return + + update_prices = self._update_prices + _global_update_prices = None + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + try: + update_prices._stop_thread() # pyright: ignore[reportPrivateUsage] + except Exception as e: + logger.error('Error from genai-prices background updater while closing (%s): %s', type(e).__name__, e) @dataclass @@ -113,27 +178,34 @@ def start(self, *, wait: bool | float = False): """ global _global_update_prices - if self._thread is not None: - raise RuntimeError('UpdatePrices background task already started') + with _global_update_prices_lock: + if self._thread is not None: + raise RuntimeError('UpdatePrices background task already started') - # Maybe we need to change up a few things right here? - # We cannot keep raising here because we do need multiple threads to be able to at least call it - # Whether we start the updater is a different thing + if _global_update_prices is not None: + raise RuntimeError( + 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' + ) - if _global_update_prices is not None: - raise RuntimeError( - 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' - ) - return + _global_update_prices = self + try: + self._start_thread() + except Exception: + _global_update_prices = None + raise + + if wait: + self.wait(timeout=30 if wait is True else wait) + + def _start_thread(self) -> None: + if self._thread is not None: + raise RuntimeError('UpdatePrices background task already started') - _global_update_prices = self self._prices_updated.clear() self._stop_event.clear() self._background_exc = None self._thread = threading.Thread(target=self._background_task, daemon=True, name='genai_prices:update') self._thread.start() - if wait: - self.wait(timeout=30 if wait is True else wait) def wait(self, timeout: float | None = None) -> bool: """Wait for the prices to be updated in the background task. @@ -150,9 +222,20 @@ def wait(self, timeout: float | None = None) -> bool: def stop(self): """Stop the background task.""" - global _global_update_prices + global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count + + with _global_update_prices_lock: + if self._thread is None: + return + + if _global_update_prices is self: + _global_update_prices = None + if _managed_update_prices is self: + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + self._stop_thread() - _global_update_prices = None + def _stop_thread(self) -> None: if self._thread is not None: self._stop_event.set() self._thread.join() diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 47df1111..5fc83e4e 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -7,13 +7,16 @@ from genai_prices import ( UpdatePrices, + UpdatePricesHandle, Usage, calc_price, data_snapshot, + update_prices as update_prices_module, update_prices_in_background, wait_prices_updated_async, wait_prices_updated_sync, ) +from genai_prices.update_prices import DEFAULT_UPDATE_URL pytestmark = pytest.mark.anyio @@ -35,7 +38,7 @@ def raise_for_status(self) -> None: def fake_get(url: str, timeout: httpx2.Timeout) -> Response: assert url in { 'https://example.test/prices.json', - 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json', + DEFAULT_UPDATE_URL, } assert timeout is not None return Response(content) @@ -181,19 +184,105 @@ def test_update_prices_multiple(monkeypatch: pytest.MonkeyPatch): pass -def test_update_prices_ref_count(): - # Both should get the same global updater from genai-prices +def test_update_prices_in_background_inert_when_manual_updater_running(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + with UpdatePrices(): + handle = update_prices_in_background() + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + + handle.close() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None + + +def test_update_prices_in_background_ref_count(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) handle_1 = update_prices_in_background() handle_2 = update_prices_in_background() - assert handle_1 is not handle_2 - handle_1.close() - handle_2.close() - assert data_snapshot._custom_snapshot is not None + try: + assert handle_1 is not handle_2 + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + handle_1.close() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None + + handle_1.close() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None -def test_update_prices_manual_with_global(): + handle_2.close() + assert data_snapshot._custom_snapshot is None + assert not wait_prices_updated_sync(timeout=0) + finally: + handle_1.close() + handle_2.close() + data_snapshot.set_custom_snapshot(None) + + +def test_update_prices_in_background_conflicts_with_manual_start(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) handle = update_prices_in_background() - with UpdatePrices(): + try: + with pytest.raises( + RuntimeError, + match='UpdatePrices global task already started, only one UpdatePrices can be active at a time', + ): + with UpdatePrices(): + pass + finally: + handle.close() + data_snapshot.set_custom_snapshot(None) + + +def test_update_prices_handle_directly_constructed_is_inert(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + handle = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + + UpdatePricesHandle().close() + assert wait_prices_updated_sync(timeout=0) assert data_snapshot._custom_snapshot is not None + finally: + handle.close() + data_snapshot.set_custom_snapshot(None) + assert data_snapshot._custom_snapshot is None + + +def test_update_prices_handle_close_does_not_raise_after_failed_fetch( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +): + def fake_get(*_args: object, **_kwargs: object) -> object: + raise httpx2.ConnectError('network down') + + monkeypatch.setattr(httpx2, 'get', fake_get) + handle = update_prices_in_background() + try: + # Wait on the internal event rather than wait_prices_updated_sync, which would + # consume the stored exception that close() is expected to log instead of raise. + updater = update_prices_module._managed_update_prices + assert updater is not None + assert updater._prices_updated.wait(timeout=5) + finally: + with caplog.at_level('ERROR', logger='genai-prices'): + handle.close() + data_snapshot.set_custom_snapshot(None) + assert any('while closing' in record.message for record in caplog.records) + + +def test_update_prices_in_background_disabled_by_env_var(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('GENAI_PRICES_DISABLE_AUTO_UPDATE', '1') + + def fail_get(*_args: object, **_kwargs: object) -> object: + raise AssertionError('no network requests should be made') + + monkeypatch.setattr(httpx2, 'get', fail_get) + handle = update_prices_in_background() + assert not wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is None handle.close() - assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None From e263bd4698d5079558edde19db9de5ea2c0a91b0 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 13:39:41 +0530 Subject: [PATCH 05/17] adding a comment for blocking on join for close --- packages/python/genai_prices/update_prices.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index abaa1d87..3986961b 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -145,6 +145,10 @@ def close(self): _global_update_prices = None _managed_update_prices = None _managed_update_prices_ref_count = 0 + # _stop_thread() joins the background thread while the global lock is held, so if a fetch + # is in flight, callers contending for the lock can block for up to the request timeout. + # This is deliberate: joining outside the lock would let a new updater start while the old + # thread can still install or clear the snapshot, requiring snapshot-ownership tracking. try: update_prices._stop_thread() # pyright: ignore[reportPrivateUsage] except Exception as e: From 58ac105162d4b88d0f9889f524acaa9b25673d99 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 14:20:41 +0530 Subject: [PATCH 06/17] Let a manually started UpdatePrices take over the shared background updater A manually started UpdatePrices now takes precedence over the shared updater from update_prices_in_background() regardless of start order: start() stops the shared updater and takes over instead of raising RuntimeError, and existing handles become inert. Two manually created instances still conflict loudly. Both precedence paths (takeover, and deferring to an already-running manual updater) are logged at INFO on the existing genai-prices logger, matching the module's lifecycle-at-INFO convention. Also documents the known fork limitation (e.g. gunicorn preload_app): the module-level updater state does not survive os.fork(), so a forked worker re-enabling background updates never starts a new thread. Co-Authored-By: Claude Fable 5 --- packages/python/README.md | 20 ++++++-- packages/python/genai_prices/update_prices.py | 47 ++++++++++++++++--- tests/test_update_prices.py | 44 ++++++++++++++--- 3 files changed, 92 insertions(+), 19 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index 1769fa3c..d25f21dc 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -142,11 +142,21 @@ the shared updater is not configurable; if you need a custom URL or interval, us calls reuse the same updater and return independent handles. The updater is stopped when the last handle is closed, at which point prices revert to the data bundled with the installed package. -If an `UpdatePrices` instance has already been started manually, `update_prices_in_background()` does not start a -second updater and returns a handle that does nothing on close: prices are already being kept up to date, and the -manual updater's lifetime stays with whoever started it. Note that such a handle stays inert: if the manual -updater is later stopped, background updates stop with it — call `update_prices_in_background()` again to start a -new shared updater. +A manually started `UpdatePrices` always takes precedence over the shared updater, regardless of which started +first: + +- If an `UpdatePrices` instance has already been started manually, `update_prices_in_background()` does not start + a second updater and returns a handle that does nothing on close: prices are already being kept up to date, and + the manual updater's lifetime stays with whoever started it. +- If `UpdatePrices.start()` is called while the shared updater is running, the shared updater is stopped and the + manual instance takes over; existing handles become inert. Prices briefly revert to the bundled data until the + manual updater's first fetch completes — pass `wait` to `start()` to block until then. + +Both cases are logged at `INFO` level on the `genai-prices` logger, which is the place to look if background +updates ever stop unexpectedly. + +Either way, an inert handle stays inert: if the manual updater is later stopped, background updates stop with +it — call `update_prices_in_background()` again to start a new shared updater. `UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged instead. diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 3986961b..93b39d69 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -22,6 +22,10 @@ logger = logging.getLogger('genai-prices') DEFAULT_UPDATE_URL = 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json' +# TODO: this module-level state does not survive os.fork() (e.g. gunicorn with preload_app=True): +# the child inherits bookkeeping claiming an updater is running, but the updater thread is dead, so +# update_prices_in_background() in a forked worker never starts a new thread and prices stay frozen +# at the fork-time snapshot. Consider resetting this state via os.register_at_fork(after_in_child=...). _global_update_prices: UpdatePrices | None = None _managed_update_prices: UpdatePrices | None = None _managed_update_prices_ref_count = 0 @@ -69,10 +73,12 @@ def update_prices_in_background() -> UpdatePricesHandle: are not recalculated once fresh data lands. Use `wait_prices_updated_sync` or `wait_prices_updated_async` if you need fresh prices before calculating. - If an `UpdatePrices` instance has already been started manually, no second updater is started - and the returned handle does nothing on close — prices are already being kept up to date. Such - a handle stays inert: if the manual updater is later stopped, background updates stop with it, - and a new handle must be acquired to restart them. + A manually started `UpdatePrices` always takes precedence over the shared updater. If one is + already running, no second updater is started and the returned handle does nothing on close — + prices are already being kept up to date. If one is started later, the shared updater is + stopped and existing handles become inert. Either way, an inert handle stays inert: if the + manual updater is later stopped, background updates stop with it, and a new handle must be + acquired to restart them. Set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value to disable this function entirely: it returns a do-nothing handle and no updater is started. @@ -93,6 +99,10 @@ def update_prices_in_background() -> UpdatePricesHandle: if _global_update_prices is not None: # A manually started UpdatePrices is already keeping prices fresh; don't start a # second updater and don't claim the manual one — its owner controls its lifetime. + logger.info( + 'A manually started UpdatePrices is already running; update_prices_in_background() ' + 'is returning an inert handle and not starting a shared updater.' + ) return UpdatePricesHandle() update_prices = UpdatePrices() @@ -176,20 +186,43 @@ class UpdatePrices: def start(self, *, wait: bool | float = False): """Start the background task. + If the shared background updater started by `update_prices_in_background` is running, it is + stopped and this instance takes over: a manually started `UpdatePrices` always takes + precedence, and existing handles on the shared updater become inert. Prices briefly revert + to the bundled data until this instance's first fetch completes (pass `wait` to block until + then). Starting a second manually created `UpdatePrices` still raises `RuntimeError`. + Args: wait: Whether to wait for the prices to be updated before returning, if an int is passed wait for that many seconds, if `True` wait for 30 seconds. """ - global _global_update_prices + global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count with _global_update_prices_lock: if self._thread is not None: raise RuntimeError('UpdatePrices background task already started') if _global_update_prices is not None: - raise RuntimeError( - 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' + if _global_update_prices is not _managed_update_prices: + raise RuntimeError( + 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' + ) + + managed = _global_update_prices + logger.info( + 'Stopping the shared background updater started via update_prices_in_background(); ' + 'its open handles (e.g. held by libraries such as logfire) are now inert and this ' + 'manually started UpdatePrices takes over.' ) + _global_update_prices = None + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + try: + managed._stop_thread() + except Exception as e: + logger.error( + 'Error from genai-prices background updater while taking over (%s): %s', type(e).__name__, e + ) _global_update_prices = self try: diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 5fc83e4e..d27e3890 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -223,21 +223,51 @@ def test_update_prices_in_background_ref_count(monkeypatch: pytest.MonkeyPatch): data_snapshot.set_custom_snapshot(None) -def test_update_prices_in_background_conflicts_with_manual_start(monkeypatch: pytest.MonkeyPatch): +def test_manual_start_takes_over_shared_updater(monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture): _mock_update_prices_get(monkeypatch) handle = update_prices_in_background() try: - with pytest.raises( - RuntimeError, - match='UpdatePrices global task already started, only one UpdatePrices can be active at a time', - ): - with UpdatePrices(): - pass + assert wait_prices_updated_sync(timeout=5) + with caplog.at_level('INFO', logger='genai-prices'): + with UpdatePrices() as manual: + manual.wait(timeout=5) + assert data_snapshot._custom_snapshot is not None + assert update_prices_module._global_update_prices is manual + assert update_prices_module._managed_update_prices is None + + # the pre-takeover handle is inert: closing it does not stop the manual updater + handle.close() + assert data_snapshot._custom_snapshot is not None + assert update_prices_module._global_update_prices is manual + assert any('takes over' in record.message for record in caplog.records) + assert data_snapshot._custom_snapshot is None + assert not [t for t in threading.enumerate() if t.name == 'genai_prices:update'] finally: handle.close() data_snapshot.set_custom_snapshot(None) +def test_background_updater_restarts_after_manual_takeover_stops(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + handle_1 = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + with UpdatePrices(): + pass + assert data_snapshot._custom_snapshot is None + + handle_2 = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + finally: + handle_2.close() + assert data_snapshot._custom_snapshot is None + finally: + handle_1.close() + data_snapshot.set_custom_snapshot(None) + + def test_update_prices_handle_directly_constructed_is_inert(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) handle = update_prices_in_background() From 5856e2e06dbd223c79bfe19fe721e2af7846a93c Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 14:24:33 +0530 Subject: [PATCH 07/17] Tighten README ordering for shared-updater precedence docs Co-Authored-By: Claude Fable 5 --- packages/python/README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index d25f21dc..4a20ca84 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -152,11 +152,10 @@ first: manual instance takes over; existing handles become inert. Prices briefly revert to the bundled data until the manual updater's first fetch completes — pass `wait` to `start()` to block until then. -Both cases are logged at `INFO` level on the `genai-prices` logger, which is the place to look if background -updates ever stop unexpectedly. - Either way, an inert handle stays inert: if the manual updater is later stopped, background updates stop with -it — call `update_prices_in_background()` again to start a new shared updater. +it — call `update_prices_in_background()` again to start a new shared updater. Both precedence cases are logged +at `INFO` level on the `genai-prices` logger, which is the place to look if background updates ever stop +unexpectedly. `UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged instead. From 8da41446cf933c9cfb43609d17662d856ccf904d Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 15:11:19 +0530 Subject: [PATCH 08/17] Harden wait semantics and close review gaps in the shared updater Address the confirmed findings from an adversarially-verified review of this PR: - wait_prices_updated_sync/async never raise and return False until a fetch actually succeeds (new _update_succeeded flag): previously the first waiter got the raw fetch exception and later waiters got True with bundled data still active. The stored exception is no longer consumed by module-level waits, so it still surfaces exactly once to the updater's owner via wait()/stop(), or in close()'s log. - UpdatePrices.wait() no longer returns True when the only completed fetch attempt failed. - Document the blocking window: close()/takeover join the updater thread under the global lock, so lifecycle calls can block for roughly the request timeout mid-fetch (calc_price is unaffected). - Extend the fork TODO with the inherited-lock deadlock mode; warn against copying handles; document stop() as a no-op when unstarted. - New tests: 16-thread acquire/close stress, stale handle closed while a different live updater runs, failed-fetch wait semantics, env var not affecting manual updaters, stop() on an unstarted instance, and the inert-handle INFO log. Co-Authored-By: Claude Fable 5 --- packages/python/README.md | 24 ++-- packages/python/genai_prices/update_prices.py | 71 ++++++++--- tests/test_update_prices.py | 117 +++++++++++++++++- 3 files changed, 187 insertions(+), 25 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index 4a20ca84..937579ec 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -157,14 +157,22 @@ it — call `update_prices_in_background()` again to start a new shared updater. at `INFO` level on the `genai-prices` logger, which is the place to look if background updates ever stop unexpectedly. -`UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged instead. - -`update_prices_in_background()` returns immediately and never blocks on the download. Until the first fetch -completes, `calc_price` keeps using the data bundled with the installed package, so prices for models released -after that snapshot may be missing for the first moments of the process. Once the fetch lands, every subsequent -calculation uses the fresh data — prices computed before then are not recalculated. If you need fresh prices -before calculating (e.g. in a short-lived script), call `wait_prices_updated_sync()` / -`wait_prices_updated_async()` after acquiring the handle. +`UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged +instead. Closing the last handle stops the updater and waits for its thread to exit, so if a fetch is in flight, +`close()` (and `UpdatePrices.start()` when taking over) can block for roughly the request timeout (typically +10–15 seconds with the default settings, since httpx timeouts are per-operation rather than a total deadline). +Other updater lifecycle calls (`update_prices_in_background()`, `wait_prices_updated_sync()`) contend +on the same internal lock and can block for the same duration in that window; `calc_price` never takes that lock +and is unaffected. A handle represents exactly one claim on the updater — don't copy one, as closing the copy +releases the original's claim too. + +`update_prices_in_background()` does not wait for the download. Until the first fetch completes, `calc_price` +keeps using the data bundled with the installed package, so prices for models released after that snapshot may be +missing for the first moments of the process. Once the fetch lands, every subsequent calculation uses the fresh +data — prices computed before then are not recalculated. If you need fresh prices before calculating (e.g. in a +short-lived script), call `wait_prices_updated_sync()` / `wait_prices_updated_async()` after acquiring the +handle — these never raise and return `False` if the update failed (the error is logged on the `genai-prices` +logger), so your calculations simply fall back to the bundled data. To disable background updates entirely (e.g. in air-gapped environments, or when a library enables them on your behalf), set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value: diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 93b39d69..252f9a9f 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -22,10 +22,14 @@ logger = logging.getLogger('genai-prices') DEFAULT_UPDATE_URL = 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json' -# TODO: this module-level state does not survive os.fork() (e.g. gunicorn with preload_app=True): -# the child inherits bookkeeping claiming an updater is running, but the updater thread is dead, so -# update_prices_in_background() in a forked worker never starts a new thread and prices stay frozen -# at the fork-time snapshot. Consider resetting this state via os.register_at_fork(after_in_child=...). +# TODO: this module-level state (including the lock) does not survive os.fork() (e.g. gunicorn with +# preload_app=True): the child inherits bookkeeping claiming an updater is running, but the updater +# thread is dead, so update_prices_in_background() in a forked worker never starts a new thread and +# prices stay frozen at the fork-time snapshot. Worse, if the fork lands while another thread holds +# _global_update_prices_lock (e.g. close() joining an in-flight fetch), the child inherits the lock +# permanently held and every lock-taking updater API call in it deadlocks (calc_price and +# UpdatePrices.wait() never take this lock). Consider resetting this state, and reinitializing the +# lock, via os.register_at_fork(after_in_child=...). _global_update_prices: UpdatePrices | None = None _managed_update_prices: UpdatePrices | None = None _managed_update_prices_ref_count = 0 @@ -35,28 +39,39 @@ def wait_prices_updated_sync(timeout: float | None = None) -> bool: """Synchronously wait for prices to be updated. + Never raises: if the update attempt failed, this returns False — the error is logged on the + `genai-prices` logger, and for a manually started `UpdatePrices` it is also re-raised from + `UpdatePrices.wait()` and `UpdatePrices.stop()`. + Args: - timeout: The maximum time to wait for prices to be updated. Defaults to None which waits indefinitely. + timeout: The maximum time to wait for prices to be updated. Defaults to None which waits + indefinitely. The timeout covers waiting on the update itself; if the updater is + concurrently being stopped mid-fetch, this call can additionally block on an internal + lock for roughly the request timeout before it starts waiting. Returns: - True if prices were updated, False otherwise. + True if prices were updated, False otherwise (including when the update failed). """ with _global_update_prices_lock: update_prices = _global_update_prices if update_prices is not None: - return update_prices.wait(timeout) + return update_prices._wait_updated(timeout) # pyright: ignore[reportPrivateUsage] return False async def wait_prices_updated_async(timeout: float | None = None) -> bool: """Asynchronously wait for prices to be updated. + Never raises: if the update attempt failed, this returns False — the error is logged on the + `genai-prices` logger, and for a manually started `UpdatePrices` it is also re-raised from + `UpdatePrices.wait()` and `UpdatePrices.stop()`. + Args: timeout: The maximum time to wait for prices to be updated. Defaults to None which waits indefinitely. Returns: - True if prices were updated, False otherwise. + True if prices were updated, False otherwise (including when the update failed). """ return await asyncio.to_thread(wait_prices_updated_sync, timeout) @@ -68,10 +83,12 @@ def update_prices_in_background() -> UpdatePricesHandle: The updater is stopped when the last handle is closed, at which point prices revert to the data bundled with the installed package. - This function returns immediately and never blocks on the download: until the first fetch - completes, price calculations keep using the bundled data, and prices computed before then - are not recalculated once fresh data lands. Use `wait_prices_updated_sync` or - `wait_prices_updated_async` if you need fresh prices before calculating. + This function does not wait for the download: until the first fetch completes, price + calculations keep using the bundled data, and prices computed before then are not recalculated + once fresh data lands. Use `wait_prices_updated_sync` or `wait_prices_updated_async` if you + need fresh prices before calculating. (Like all updater lifecycle calls, it can briefly block + on an internal lock — for roughly the request timeout — if another thread is concurrently + stopping an updater whose fetch is in flight.) A manually started `UpdatePrices` always takes precedence over the shared updater. If one is already running, no second updater is started and the returned handle does nothing on close — @@ -126,6 +143,10 @@ class UpdatePricesHandle: A handle only releases the updater it was created for: handles constructed directly, or outliving the updater they belong to, are inert and closing them does nothing. + + Do not copy a handle (`copy.copy`, `dataclasses.replace`, pickling): each handle represents + exactly one claim, so closing a copy releases the original's claim and can stop the shared + updater while other consumers still hold open handles. """ _update_prices: UpdatePrices | None = None @@ -136,6 +157,10 @@ def close(self): Stops the updater if this was the last open handle. Idempotent, and never raises: errors from the background updater are logged instead. + + Stopping the updater waits for its thread to exit, so closing the last handle can block + for roughly the request timeout (typically 10-15s with the default settings, since httpx + timeouts are per-operation rather than a total deadline) if a fetch is in flight. """ global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count @@ -182,6 +207,7 @@ class UpdatePrices: _prices_updated: threading.Event = field(default_factory=threading.Event) _thread: threading.Thread | None = field(default=None, init=False) _background_exc: Exception | None = field(default=None, init=False) + _update_succeeded: bool = field(default=False, init=False) def start(self, *, wait: bool | float = False): """Start the background task. @@ -241,12 +267,15 @@ def _start_thread(self) -> None: self._prices_updated.clear() self._stop_event.clear() self._background_exc = None + self._update_succeeded = False self._thread = threading.Thread(target=self._background_task, daemon=True, name='genai_prices:update') self._thread.start() def wait(self, timeout: float | None = None) -> bool: """Wait for the prices to be updated in the background task. + Raises the stored exception if the update attempt failed. + Args: timeout: The maximum time to wait for the prices to be updated in seconds. """ @@ -255,10 +284,20 @@ def wait(self, timeout: float | None = None) -> bool: if exc: self._background_exc = None raise exc - return prices_updated + return prices_updated and self._update_succeeded + + def _wait_updated(self, timeout: float | None) -> bool: + # Unlike wait(), never raises and does not consume the stored background exception: + # failures are already logged by the background task, and the exception stays stored for + # the updater's owner to surface via wait()/stop()/close(). + return self._prices_updated.wait(timeout=timeout) and self._update_succeeded def stop(self): - """Stop the background task.""" + """Stop the background task. + + A no-op if this instance was never started or has already been stopped; it does not + affect any other updater that happens to be running. + """ global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count with _global_update_prices_lock: @@ -297,8 +336,10 @@ def _background_task(self) -> None: while True: try: self._update_prices() - self._prices_updated.set() self._background_exc = None + # Set before signaling the event so waiters observing the event see the flag. + self._update_succeeded = True + self._prices_updated.set() except Exception as e: self._background_exc = e self._prices_updated.set() diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index d27e3890..1095bfdc 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -184,10 +184,14 @@ def test_update_prices_multiple(monkeypatch: pytest.MonkeyPatch): pass -def test_update_prices_in_background_inert_when_manual_updater_running(monkeypatch: pytest.MonkeyPatch): +def test_update_prices_in_background_inert_when_manual_updater_running( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +): _mock_update_prices_get(monkeypatch) with UpdatePrices(): - handle = update_prices_in_background() + with caplog.at_level('INFO', logger='genai-prices'): + handle = update_prices_in_background() + assert any('returning an inert handle' in record.message for record in caplog.records) assert wait_prices_updated_sync(timeout=5) assert data_snapshot._custom_snapshot is not None @@ -316,3 +320,112 @@ def fail_get(*_args: object, **_kwargs: object) -> object: assert data_snapshot._custom_snapshot is None handle.close() assert data_snapshot._custom_snapshot is None + + +def test_disable_env_var_does_not_affect_manual_update_prices(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv('GENAI_PRICES_DISABLE_AUTO_UPDATE', '1') + _mock_update_prices_get(monkeypatch) + with UpdatePrices() as update_prices: + assert update_prices.wait(timeout=5) + assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None + + +def test_wait_prices_updated_sync_returns_false_on_failed_fetch( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +): + def fake_get(*_args: object, **_kwargs: object) -> object: + raise httpx2.ConnectError('network down') + + monkeypatch.setattr(httpx2, 'get', fake_get) + handle = update_prices_in_background() + try: + # Never raises and returns False on failure, for every waiter — not just the first. + assert wait_prices_updated_sync(timeout=5) is False + assert wait_prices_updated_sync(timeout=0) is False + assert data_snapshot._custom_snapshot is None + finally: + with caplog.at_level('ERROR', logger='genai-prices'): + handle.close() + data_snapshot.set_custom_snapshot(None) + # The stored exception is not consumed by the waiters above, so close() still logs it. + assert any('while closing' in record.message for record in caplog.records) + + +def test_stale_handle_close_does_not_affect_new_shared_updater(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + handle_1 = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + with UpdatePrices(): + pass + + handle_2 = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + new_updater = update_prices_module._managed_update_prices + assert new_updater is not None + + # handle_1 is bound to the pre-takeover updater: closing it while a different + # shared updater is live must not release handle_2's claim. + handle_1.close() + assert update_prices_module._managed_update_prices is new_updater + assert update_prices_module._managed_update_prices_ref_count == 1 + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None + finally: + handle_2.close() + assert data_snapshot._custom_snapshot is None + finally: + handle_1.close() + data_snapshot.set_custom_snapshot(None) + + +def test_stop_on_unstarted_instance_is_noop(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + with UpdatePrices() as update_prices: + assert update_prices.wait(timeout=5) + UpdatePrices().stop() + assert update_prices_module._global_update_prices is update_prices + assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None + + +def test_update_prices_in_background_concurrent_acquire_close(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + barrier = threading.Barrier(16) + errors: list[BaseException] = [] + + def worker() -> None: + try: + barrier.wait(timeout=5) + for _ in range(10): + handle = update_prices_in_background() + handle.close() + except BaseException as exc: # pragma: no cover + errors.append(exc) + + threads = [threading.Thread(target=worker) for _ in range(16)] + try: + for thread in threads: + thread.start() + finally: + for thread in threads: + thread.join(timeout=30) + + assert not [t for t in threads if t.is_alive()] + assert not errors + assert update_prices_module._global_update_prices is None + assert update_prices_module._managed_update_prices is None + assert update_prices_module._managed_update_prices_ref_count == 0 + assert not [t for t in threading.enumerate() if t.name == 'genai_prices:update'] + assert data_snapshot._custom_snapshot is None + + # The shared updater can still be acquired cleanly after the churn. + handle = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + finally: + handle.close() + data_snapshot.set_custom_snapshot(None) From 36b595f57e03083176d10bb51026faf68e60945f Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 17:15:12 +0530 Subject: [PATCH 09/17] Remove the GENAI_PRICES_DISABLE_AUTO_UPDATE kill switch Both planned consumers (logfire, pydantic-ai) expose their own opt-in flags, so a library-side opt-out has no user while auto-update is not on by default. It can be reintroduced if defaults ever flip. Co-Authored-By: Claude Fable 5 --- packages/python/README.md | 5 ---- packages/python/genai_prices/update_prices.py | 7 ------ tests/test_update_prices.py | 23 ------------------- 3 files changed, 35 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index 937579ec..eeddfe19 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -174,11 +174,6 @@ short-lived script), call `wait_prices_updated_sync()` / `wait_prices_updated_as handle — these never raise and return `False` if the update failed (the error is logged on the `genai-prices` logger), so your calculations simply fall back to the bundled data. -To disable background updates entirely (e.g. in air-gapped environments, or when a library enables them on your -behalf), set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value: -`update_prices_in_background()` then returns a do-nothing handle and makes no network requests. This does not -affect manually created `UpdatePrices` instances. - If you'd like to wait for prices to be updated without access to the `UpdatePrices` instance, you can use the `wait_prices_updated_sync` function: ```py diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 252f9a9f..f23fc54d 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -2,7 +2,6 @@ import asyncio import logging -import os import threading from dataclasses import dataclass, field from time import time @@ -97,17 +96,11 @@ def update_prices_in_background() -> UpdatePricesHandle: manual updater is later stopped, background updates stop with it, and a new handle must be acquired to restart them. - Set the `GENAI_PRICES_DISABLE_AUTO_UPDATE` environment variable to any non-empty value to - disable this function entirely: it returns a do-nothing handle and no updater is started. - Returns: A handle that stops the shared background updater when all handles have been closed. """ global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count - if os.environ.get('GENAI_PRICES_DISABLE_AUTO_UPDATE'): - return UpdatePricesHandle() - with _global_update_prices_lock: if _managed_update_prices is not None: _managed_update_prices_ref_count += 1 diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 1095bfdc..fa37262f 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -308,29 +308,6 @@ def fake_get(*_args: object, **_kwargs: object) -> object: assert any('while closing' in record.message for record in caplog.records) -def test_update_prices_in_background_disabled_by_env_var(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setenv('GENAI_PRICES_DISABLE_AUTO_UPDATE', '1') - - def fail_get(*_args: object, **_kwargs: object) -> object: - raise AssertionError('no network requests should be made') - - monkeypatch.setattr(httpx2, 'get', fail_get) - handle = update_prices_in_background() - assert not wait_prices_updated_sync(timeout=0) - assert data_snapshot._custom_snapshot is None - handle.close() - assert data_snapshot._custom_snapshot is None - - -def test_disable_env_var_does_not_affect_manual_update_prices(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setenv('GENAI_PRICES_DISABLE_AUTO_UPDATE', '1') - _mock_update_prices_get(monkeypatch) - with UpdatePrices() as update_prices: - assert update_prices.wait(timeout=5) - assert data_snapshot._custom_snapshot is not None - assert data_snapshot._custom_snapshot is None - - def test_wait_prices_updated_sync_returns_false_on_failed_fetch( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ): From 8ca8cb5287d2d19b71aaf6271e459a08b7b72f70 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 18:43:35 +0530 Subject: [PATCH 10/17] Make stopping the shared updater non-blocking with a fencing gate Previously UpdatePricesHandle.close() (and takeover in start()) joined the background thread while holding the global lock, so a fetch in flight could block the closer - and every other updater API call - for roughly the request timeout (10-15s). That cost lands directly on consumers: logfire calls close() from configure() and atexit. Following the pattern used by LaunchDarkly FDv2, statsig, and OpenTelemetry (join with a short timeout outside any lock, then abandon the daemon thread), and LaunchDarkly FDv2's stop-flag fencing gate before publishing: - Snapshot installs are gated: the background thread re-checks _stop_event under a new _snapshot_lock before calling set_custom_snapshot, and the stop path sets the event and clears the snapshot under the same lock, so a draining fetch can never install its result after stop. - close() and takeover now detach under the global lock (bookkeeping only), then join with a 5s grace period outside it, logging a warning and abandoning the daemon thread on expiry. - Manual UpdatePrices.stop() keeps its unbounded join (owners expect completion and the re-raised stored exception) but joins outside the global lock, so it no longer blocks unrelated API calls. httpx requests cannot be aborted cross-thread (encode/httpx#1633), so the in-flight fetch itself remains bounded only by the request timeout. Co-Authored-By: Claude Fable 5 --- packages/python/README.md | 14 +-- packages/python/genai_prices/update_prices.py | 118 +++++++++++++----- tests/test_update_prices.py | 46 +++++++ 3 files changed, 138 insertions(+), 40 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index eeddfe19..7a244891 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -158,13 +158,13 @@ at `INFO` level on the `genai-prices` logger, which is the place to look if back unexpectedly. `UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged -instead. Closing the last handle stops the updater and waits for its thread to exit, so if a fetch is in flight, -`close()` (and `UpdatePrices.start()` when taking over) can block for roughly the request timeout (typically -10–15 seconds with the default settings, since httpx timeouts are per-operation rather than a total deadline). -Other updater lifecycle calls (`update_prices_in_background()`, `wait_prices_updated_sync()`) contend -on the same internal lock and can block for the same duration in that window; `calc_price` never takes that lock -and is unaffected. A handle represents exactly one claim on the updater — don't copy one, as closing the copy -releases the original's claim too. +instead. Closing the last handle stops the updater and reverts prices to the bundled data immediately. If a +fetch is in flight, the daemon thread is given a short grace period to exit and is otherwise abandoned with a +warning log: it exits once the fetch completes, and its result is discarded — a stopped updater can never +install prices afterwards. (`UpdatePrices.stop()` instead waits for the thread to exit, so a manual stop can +block its caller for roughly the request timeout; neither blocks other updater API calls, and `calc_price` +is always unaffected.) A handle represents exactly one claim on the updater — don't copy one, as closing the +copy releases the original's claim too. `update_prices_in_background()` does not wait for the download. Until the first fetch completes, `calc_price` keeps using the data bundled with the installed package, so prices for models released after that snapshot may be diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index f23fc54d..66083172 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -34,6 +34,28 @@ _managed_update_prices_ref_count = 0 _global_update_prices_lock = threading.RLock() +_STOPPED_THREAD_JOIN_TIMEOUT = 5.0 + + +def _join_stopped_updater_thread(thread: threading.Thread | None) -> None: + """Give a stopped updater thread a short grace period to exit, then abandon it. + + Called outside `_global_update_prices_lock` so a thread blocked on an in-flight fetch never + stalls other updater API calls. Abandonment is safe: the thread is a daemon, it exits as soon + as the fetch completes, and a stopped updater can never install its result (see + `UpdatePrices._install_snapshot`). + """ + if thread is None: + return + thread.join(timeout=_STOPPED_THREAD_JOIN_TIMEOUT) + if thread.is_alive(): + logger.warning( + 'genai-prices background updater thread did not exit within %.0f seconds (a fetch is ' + 'likely in flight); abandoning the daemon thread. It will exit once the fetch ' + 'completes, without updating prices.', + _STOPPED_THREAD_JOIN_TIMEOUT, + ) + def wait_prices_updated_sync(timeout: float | None = None) -> bool: """Synchronously wait for prices to be updated. @@ -44,9 +66,7 @@ def wait_prices_updated_sync(timeout: float | None = None) -> bool: Args: timeout: The maximum time to wait for prices to be updated. Defaults to None which waits - indefinitely. The timeout covers waiting on the update itself; if the updater is - concurrently being stopped mid-fetch, this call can additionally block on an internal - lock for roughly the request timeout before it starts waiting. + indefinitely. Returns: True if prices were updated, False otherwise (including when the update failed). @@ -85,9 +105,7 @@ def update_prices_in_background() -> UpdatePricesHandle: This function does not wait for the download: until the first fetch completes, price calculations keep using the bundled data, and prices computed before then are not recalculated once fresh data lands. Use `wait_prices_updated_sync` or `wait_prices_updated_async` if you - need fresh prices before calculating. (Like all updater lifecycle calls, it can briefly block - on an internal lock — for roughly the request timeout — if another thread is concurrently - stopping an updater whose fetch is in flight.) + need fresh prices before calculating. A manually started `UpdatePrices` always takes precedence over the shared updater. If one is already running, no second updater is started and the returned handle does nothing on close — @@ -151,9 +169,9 @@ def close(self): Stops the updater if this was the last open handle. Idempotent, and never raises: errors from the background updater are logged instead. - Stopping the updater waits for its thread to exit, so closing the last handle can block - for roughly the request timeout (typically 10-15s with the default settings, since httpx - timeouts are per-operation rather than a total deadline) if a fetch is in flight. + Returns promptly: prices revert to the bundled data immediately, and if a fetch is in + flight the daemon thread is given a short grace period to exit before being abandoned + with a warning log - a stopped updater can never install its result afterwards. """ global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count @@ -173,14 +191,15 @@ def close(self): _global_update_prices = None _managed_update_prices = None _managed_update_prices_ref_count = 0 - # _stop_thread() joins the background thread while the global lock is held, so if a fetch - # is in flight, callers contending for the lock can block for up to the request timeout. - # This is deliberate: joining outside the lock would let a new updater start while the old - # thread can still install or clear the snapshot, requiring snapshot-ownership tracking. - try: - update_prices._stop_thread() # pyright: ignore[reportPrivateUsage] - except Exception as e: - logger.error('Error from genai-prices background updater while closing (%s): %s', type(e).__name__, e) + thread = update_prices._stop_and_detach() # pyright: ignore[reportPrivateUsage] + + # Join outside the lock so a thread blocked on an in-flight fetch never stalls other + # updater API calls; re-publication is already fenced off by _stop_and_detach(). + _join_stopped_updater_thread(thread) + exc = update_prices._background_exc # pyright: ignore[reportPrivateUsage] + if exc: + update_prices._background_exc = None # pyright: ignore[reportPrivateUsage] + logger.error('Error from genai-prices background updater while closing (%s): %s', type(exc).__name__, exc) @dataclass @@ -198,6 +217,7 @@ class UpdatePrices: """The timeout for HTTP requests.""" _stop_event: threading.Event = field(default_factory=threading.Event) _prices_updated: threading.Event = field(default_factory=threading.Event) + _snapshot_lock: threading.Lock = field(default_factory=threading.Lock, init=False) _thread: threading.Thread | None = field(default=None, init=False) _background_exc: Exception | None = field(default=None, init=False) _update_succeeded: bool = field(default=False, init=False) @@ -217,6 +237,8 @@ def start(self, *, wait: bool | float = False): """ global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count + taken_over: UpdatePrices | None = None + drained_thread: threading.Thread | None = None with _global_update_prices_lock: if self._thread is not None: raise RuntimeError('UpdatePrices background task already started') @@ -227,7 +249,7 @@ def start(self, *, wait: bool | float = False): 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' ) - managed = _global_update_prices + taken_over = _global_update_prices logger.info( 'Stopping the shared background updater started via update_prices_in_background(); ' 'its open handles (e.g. held by libraries such as logfire) are now inert and this ' @@ -236,12 +258,7 @@ def start(self, *, wait: bool | float = False): _global_update_prices = None _managed_update_prices = None _managed_update_prices_ref_count = 0 - try: - managed._stop_thread() - except Exception as e: - logger.error( - 'Error from genai-prices background updater while taking over (%s): %s', type(e).__name__, e - ) + drained_thread = taken_over._stop_and_detach() _global_update_prices = self try: @@ -250,6 +267,15 @@ def start(self, *, wait: bool | float = False): _global_update_prices = None raise + if taken_over is not None: + _join_stopped_updater_thread(drained_thread) + exc = taken_over._background_exc + if exc: + taken_over._background_exc = None + logger.error( + 'Error from genai-prices background updater while taking over (%s): %s', type(exc).__name__, exc + ) + if wait: self.wait(timeout=30 if wait is True else wait) @@ -290,6 +316,10 @@ def stop(self): A no-op if this instance was never started or has already been stopped; it does not affect any other updater that happens to be running. + + Prices revert to the bundled data immediately. The call then waits for the background + thread to exit (bounded by the request timeout if a fetch is in flight) without blocking + other updater API calls, and re-raises the stored background exception, if any. """ global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count @@ -302,20 +332,32 @@ def stop(self): if _managed_update_prices is self: _managed_update_prices = None _managed_update_prices_ref_count = 0 - self._stop_thread() + thread = self._stop_and_detach() - def _stop_thread(self) -> None: - if self._thread is not None: - self._stop_event.set() - self._thread.join() - self._thread = None - # Clear after the thread exits so an in-flight fetch cannot reinstall a snapshot after stop(). - data_snapshot.set_custom_snapshot(None) + # Join outside the global lock so concurrent updater API calls don't queue behind an + # in-flight fetch; the snapshot is already reverted and re-publication fenced off. + if thread is not None: + thread.join() if self._background_exc: exc = self._background_exc self._background_exc = None raise exc + def _stop_and_detach(self) -> threading.Thread | None: + """Signal the background thread to stop and revert prices to the bundled data. + + Does not join; returns the (possibly still draining) thread so the caller can wait on it + outside `_global_update_prices_lock`. Setting the stop event before clearing the snapshot, + both ordered against `_install_snapshot` via `_snapshot_lock`, guarantees an in-flight + fetch can finish but can never install its result afterwards. + """ + thread = self._thread + self._thread = None + self._stop_event.set() + with self._snapshot_lock: + data_snapshot.set_custom_snapshot(None) + return thread + def __enter__(self): self.start() return self @@ -352,7 +394,17 @@ def _update_prices(self): else: logger.info('Successfully fetched null snapshot in %.2f seconds', interval) - data_snapshot.set_custom_snapshot(snapshot) + self._install_snapshot(snapshot) + + def _install_snapshot(self, snapshot: data_snapshot.DataSnapshot | None) -> None: + # Fencing check: never publish after stop. The stop path sets _stop_event and then clears + # the snapshot under the same lock (see _stop_and_detach), so re-checking the event here + # makes publish-after-stop impossible rather than merely unlikely. + with self._snapshot_lock: + if self._stop_event.is_set(): + logger.info('genai-prices updater was stopped during the fetch; discarding the fetched snapshot') + return + data_snapshot.set_custom_snapshot(snapshot) def fetch(self) -> data_snapshot.DataSnapshot | None: """Fetches the latest provider data from the configured URL.""" diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index fa37262f..0081347b 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -1,5 +1,6 @@ import threading from decimal import Decimal +from time import monotonic import httpx2 import pytest @@ -201,6 +202,51 @@ def test_update_prices_in_background_inert_when_manual_updater_running( assert data_snapshot._custom_snapshot is None +def test_close_does_not_block_on_in_flight_fetch_and_discards_result( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +): + fetch_started = threading.Event() + allow_fetch_return = threading.Event() + + class Response: + content = PROVIDER_ARRAY_PAYLOAD + + def raise_for_status(self) -> None: + pass + + def fake_get(url: str, timeout: httpx2.Timeout) -> Response: + assert url == DEFAULT_UPDATE_URL + assert timeout is not None + fetch_started.set() + assert allow_fetch_return.wait(timeout=5) + return Response() + + monkeypatch.setattr(httpx2, 'get', fake_get) + monkeypatch.setattr(update_prices_module, '_STOPPED_THREAD_JOIN_TIMEOUT', 0.05) + + handle = update_prices_in_background() + try: + assert fetch_started.wait(timeout=5) + (thread,) = [t for t in threading.enumerate() if t.name == 'genai_prices:update'] + + start = monotonic() + with caplog.at_level('WARNING', logger='genai-prices'): + handle.close() + assert monotonic() - start < 2 # did not wait for the in-flight fetch + assert any('abandoning the daemon thread' in record.message for record in caplog.records) + assert data_snapshot._custom_snapshot is None + + # The drained fetch completes but its snapshot is discarded by the fencing check. + allow_fetch_return.set() + thread.join(timeout=5) + assert not thread.is_alive() + assert data_snapshot._custom_snapshot is None + finally: + allow_fetch_return.set() + handle.close() + data_snapshot.set_custom_snapshot(None) + + def test_update_prices_in_background_ref_count(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) handle_1 = update_prices_in_background() From 89bcb07c64e40b304756d5384783578fa583c153 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Wed, 10 Jun 2026 18:46:03 +0530 Subject: [PATCH 11/17] Survive os.fork(): hold the lock across fork and restart the updater in the child Previously the module-level state did not survive os.fork() (e.g. gunicorn with preload_app=True): the child inherited bookkeeping claiming an updater was running while its thread was dead - so prices stayed frozen at the fork-time snapshot - and a fork landing while another thread held the global lock left it permanently held in the child, deadlocking every lock-taking updater call. With logfire starting the updater from configure(), preload+fork is a mainstream deployment path for it. Following the OpenTelemetry BatchProcessor pattern: - Fork hooks are registered lazily on first updater start. before/ after_in_parent hold the global lock across the fork so the child inherits consistent bookkeeping, never a torn mid-mutation state. - after_in_child replaces the lock and restarts a running updater on the same UpdatePrices instance - fresh events and snapshot lock, new daemon thread - preserving instance identity so handles held by consumers (logfire, pydantic-ai) remain valid claims in the child. - On any revival failure the child degrades to a clean reset (bundled prices, no deadlock) with an error log. Co-Authored-By: Claude Fable 5 --- packages/python/genai_prices/update_prices.py | 72 ++++++++++++++++--- tests/test_update_prices.py | 29 ++++++++ 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 66083172..11e7ecc0 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -2,6 +2,7 @@ import asyncio import logging +import os import threading from dataclasses import dataclass, field from time import time @@ -21,19 +22,63 @@ logger = logging.getLogger('genai-prices') DEFAULT_UPDATE_URL = 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json' -# TODO: this module-level state (including the lock) does not survive os.fork() (e.g. gunicorn with -# preload_app=True): the child inherits bookkeeping claiming an updater is running, but the updater -# thread is dead, so update_prices_in_background() in a forked worker never starts a new thread and -# prices stay frozen at the fork-time snapshot. Worse, if the fork lands while another thread holds -# _global_update_prices_lock (e.g. close() joining an in-flight fetch), the child inherits the lock -# permanently held and every lock-taking updater API call in it deadlocks (calc_price and -# UpdatePrices.wait() never take this lock). Consider resetting this state, and reinitializing the -# lock, via os.register_at_fork(after_in_child=...). +# This module-level state would not survive os.fork() on its own (threads die with the parent, +# and locks can be inherited in a held state, e.g. under gunicorn with preload_app=True). Fork +# hooks registered lazily in _register_fork_hooks() keep it consistent: the global lock is held +# across the fork and reinitialized in the child, and a running updater is restarted in place - +# see _fork_after_in_child. _global_update_prices: UpdatePrices | None = None _managed_update_prices: UpdatePrices | None = None _managed_update_prices_ref_count = 0 _global_update_prices_lock = threading.RLock() +_fork_hooks_registered = False + + +def _register_fork_hooks() -> None: + """Keep the updater working across os.fork() (e.g. gunicorn with preload_app=True). + + Called under `_global_update_prices_lock` from `_start_thread`, so registration happens at + most once, and only in processes that actually start an updater. + """ + global _fork_hooks_registered + if _fork_hooks_registered or not hasattr(os, 'register_at_fork'): + return + _fork_hooks_registered = True + os.register_at_fork(before=_fork_before, after_in_parent=_fork_after_in_parent, after_in_child=_fork_after_in_child) + + +def _fork_before() -> None: + # Hold the lock across the fork so the child inherits consistent bookkeeping rather than a + # torn, mid-mutation state. + _global_update_prices_lock.acquire() + + +def _fork_after_in_parent() -> None: + _global_update_prices_lock.release() + + +def _fork_after_in_child() -> None: + global _global_update_prices_lock, _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count + + # The child inherits the lock acquired in _fork_before; replace it with a fresh one rather + # than releasing the inherited object. + _global_update_prices_lock = threading.RLock() + updater = _global_update_prices + if updater is None: + return + try: + # The parent's updater thread does not survive the fork; restart it on the same instance, + # preserving identity so existing handles and the managed bookkeeping remain valid. + updater._revive_after_fork() # pyright: ignore[reportPrivateUsage] + except Exception as e: + _global_update_prices = None + _managed_update_prices = None + _managed_update_prices_ref_count = 0 + data_snapshot.set_custom_snapshot(None) + logger.error('Failed to restart the genai-prices background updater after fork (%s): %s', type(e).__name__, e) + + _STOPPED_THREAD_JOIN_TIMEOUT = 5.0 @@ -283,6 +328,7 @@ def _start_thread(self) -> None: if self._thread is not None: raise RuntimeError('UpdatePrices background task already started') + _register_fork_hooks() self._prices_updated.clear() self._stop_event.clear() self._background_exc = None @@ -343,6 +389,16 @@ def stop(self): self._background_exc = None raise exc + def _revive_after_fork(self) -> None: + # Threads do not survive fork, and the inherited events/locks may have been captured in an + # arbitrary state (e.g. mid-install); recreate them and restart the background thread on + # this same instance. + self._thread = None + self._stop_event = threading.Event() + self._prices_updated = threading.Event() + self._snapshot_lock = threading.Lock() + self._start_thread() + def _stop_and_detach(self) -> threading.Thread | None: """Signal the background thread to stop and revert prices to the bundled data. diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 0081347b..94d9b192 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -1,3 +1,4 @@ +import os import threading from decimal import Decimal from time import monotonic @@ -414,6 +415,34 @@ def test_stop_on_unstarted_instance_is_noop(monkeypatch: pytest.MonkeyPatch): assert data_snapshot._custom_snapshot is None +@pytest.mark.skipif(not hasattr(os, 'fork'), reason='requires os.fork') +def test_forked_child_restarts_shared_updater(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + handle = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + + pid = os.fork() + if pid == 0: + # Child: never return into pytest - report via the exit code. + try: + ok = ( + wait_prices_updated_sync(timeout=5) + and any(t.name == 'genai_prices:update' and t.is_alive() for t in threading.enumerate()) + and data_snapshot._custom_snapshot is not None + and update_prices_module._global_update_prices is not None + ) + os._exit(0 if ok else 1) + except BaseException: + os._exit(2) + + _, status = os.waitpid(pid, 0) + assert os.waitstatus_to_exitcode(status) == 0 + finally: + handle.close() + data_snapshot.set_custom_snapshot(None) + + def test_update_prices_in_background_concurrent_acquire_close(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) barrier = threading.Barrier(16) From 941f0202ef44b7f52f99710042ff28a74b0e88cb Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Thu, 18 Jun 2026 10:10:12 +0100 Subject: [PATCH 12/17] exception trace handling --- packages/python/genai_prices/update_prices.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 11e7ecc0..f4195ddd 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -71,12 +71,12 @@ def _fork_after_in_child() -> None: # The parent's updater thread does not survive the fork; restart it on the same instance, # preserving identity so existing handles and the managed bookkeeping remain valid. updater._revive_after_fork() # pyright: ignore[reportPrivateUsage] - except Exception as e: + except Exception: _global_update_prices = None _managed_update_prices = None _managed_update_prices_ref_count = 0 data_snapshot.set_custom_snapshot(None) - logger.error('Failed to restart the genai-prices background updater after fork (%s): %s', type(e).__name__, e) + logger.warning('Failed to restart the genai-prices background updater after fork', exc_info=True) _STOPPED_THREAD_JOIN_TIMEOUT = 5.0 @@ -244,7 +244,7 @@ def close(self): exc = update_prices._background_exc # pyright: ignore[reportPrivateUsage] if exc: update_prices._background_exc = None # pyright: ignore[reportPrivateUsage] - logger.error('Error from genai-prices background updater while closing (%s): %s', type(exc).__name__, exc) + logger.error('Error from genai-prices background updater while closing', exc_info=exc) @dataclass @@ -317,9 +317,7 @@ def start(self, *, wait: bool | float = False): exc = taken_over._background_exc if exc: taken_over._background_exc = None - logger.error( - 'Error from genai-prices background updater while taking over (%s): %s', type(exc).__name__, exc - ) + logger.exception('Error from genai-prices background updater while taking over', exc_info=exc) if wait: self.wait(timeout=30 if wait is True else wait) @@ -434,7 +432,7 @@ def _background_task(self) -> None: except Exception as e: self._background_exc = e self._prices_updated.set() - logger.error('Error updating genai-prices in the background (%s): %s', type(e).__name__, e) + logger.exception('Error updating genai-prices in the background') if self._stop_event.wait(self.update_interval): break From 2aef71e8adad0d40831cab7d63f910b569aa2224 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Fri, 26 Jun 2026 19:31:08 +0530 Subject: [PATCH 13/17] Collapse the background updater into a process-global singleton The instantiable UpdatePrices class let N "independent" instances all write one process-global snapshot, which forced the takeover/precedence state machine, the "only one active" RuntimeError, and a second snapshot lock. Make update_prices_in_background() the single, ref-counted entry point with first-wins config, and reduce UpdatePrices to a deprecated shim over it. Fold the snapshot fencing onto the one module lock, so the whole updater is guarded by a single RLock over a trivial state machine. Revert-on-last-close is preserved. Adds docs/adr/0001 capturing the decision and its open questions. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/adr/0001-singleton-background-updater.md | 116 +++++ packages/python/genai_prices/update_prices.py | 452 +++++++++--------- tests/test_custom_prices.py | 26 +- tests/test_tiered_pricing.py | 23 +- tests/test_update_prices.py | 310 +++++------- 5 files changed, 491 insertions(+), 436 deletions(-) create mode 100644 docs/adr/0001-singleton-background-updater.md diff --git a/docs/adr/0001-singleton-background-updater.md b/docs/adr/0001-singleton-background-updater.md new file mode 100644 index 00000000..8c929e60 --- /dev/null +++ b/docs/adr/0001-singleton-background-updater.md @@ -0,0 +1,116 @@ +# ADR 0001 — Collapse the background updater into a process-global singleton + +- Status: Proposed (for review with Alex / Samuel) +- Date: 2026-06-26 +- Context: PR #404, follow-up to the 19 Jun huddle + +## The call + +Replace the instantiable `UpdatePrices` class + per-instance `start()`/`stop()` with a +single process-global, ref-counted updater fronted by one function: + +```python +handle = genai_prices.update_prices_in_background( + *, url=..., interval=3600, timeout=..., # all optional +) +... +handle.close() # or: with update_prices_in_background() as h: ... + +genai_prices.wait_for_update(timeout=...) # sync (role unchanged) +await genai_prices.wait_for_update_async(...) # async +``` + +`UpdatePrices` is **deprecated**, not removed: it stays importable as a thin shim that +warns and routes through the singleton. It is removed in a later release. + +## Why + +There is exactly **one** price snapshot per process — `data_snapshot.set_custom_snapshot()` +is a process-global. "Updating prices" is therefore inherently a singleton activity: two +"independent" updaters would both write the same global. + +The current public API contradicts that domain fact. It exposes an instantiable class with +per-instance `start()`/`stop()`/config/context-manager, while every instance secretly shares +one global snapshot. That mismatch — N instances pretending to be independent — is the root +cause of the complexity we maintain: + +- `RuntimeError` "only one can be active at a time" +- takeover / precedence logic (manual instance preempts the shared updater) +- the duplicated entry points (`update_prices_in_background()` _and_ `UpdatePrices().start()`) +- refcount living in module globals while ownership lives on instances + +None of that is essential to the problem; it is all tax on pretending instances are independent. +Folding `update_prices_in_background()` into `start()` (the huddle's lighter option) does not +remove the tax — it just relocates the unanswered question (what happens with mismatched config +across instances) from API names into behaviour. Deleting the class answers it by construction. + +## User-facing model + +"Call `update_prices_in_background()` from anywhere, as many times as you like; close your +handle when done." Libraries (logfire, pydantic-ai) call it config-less and never close. +App authors who want a custom URL pass it, early in startup. + +Prior art: the `logging` / threading-singleton pattern — a process-global resource you +reference, not instantiate. The "configure early" rule below is the same discipline +`logging.basicConfig` teaches. + +## Config-conflict rule: first-wins + warning + +Two callers passing _different_ config (the multi-URL case) is the only real ambiguity. + +- **First-wins (chosen):** first caller's config sticks; a later caller with mismatched + config gets a warning and a handle on the already-running updater. Config-less library + calls always just join — they never conflict. App authors who want custom config call + early, before libraries initialize. +- Last-wins / takeover — rejected: reintroduces the exact state machine we are deleting. +- Raise — rejected: a library starting first would crash the app author. + +This keeps the lifecycle a monotonic state machine: empty -> configured+running -> +(refcount hits 0) -> stopped. No backward transitions, no preemption. + +## Decisions parked deliberately + +### Revert-on-close: KEEP today's behaviour + +When the last handle closes, prices revert to the package-bundled data (not the +last-fetched snapshot). The "a fetch in flight at stop time can never publish afterward" +fencing invariant therefore **stays**. We considered leaving the fresh snapshot in place +(which would let us drop the fencing entirely) but chose to preserve current semantics for +now. Revisit if/when we want that simplification. + +### Fork support: separate, stacked PR + +The `os.register_at_fork` hooks (surviving gunicorn `preload_app=True`) are orthogonal and +land as a follow-up so this change can be reviewed on its own. Under the simpler singleton +core the fork story is also easier: one thread, one config, no takeover to revive. + +## What stays / goes internally + +- A single module `_lock` (`RLock`) — **stays, and is now the only lock.** Load-bearing for + refcount + thread-handle bookkeeping (two racing first-calls must not both spawn a thread). + NOTE for the huddle's "worst case is just redundant network calls": that undersells it — + without the lock, two racing starts leak an untracked daemon thread and corrupt the refcount, + which is a correctness bug. +- The separate `_snapshot_lock` — **gone.** Its fencing job (ordering the snapshot install in + the background thread against the stop/revert) is folded onto the same module `_lock`, since + both ultimately guard the one process-global snapshot. `set_custom_snapshot` is a bare global + assignment (no internal lock, no callback), so holding `_lock` across it cannot deadlock or + invert lock order. Result: one lock guarding a trivial state machine (a pointer, an int, the + snapshot). +- Takeover / precedence / `RuntimeError` "only one active" — **gone.** +- `UpdatePrices.start()/stop()`/context-manager — **gone** (live on only via the deprecated shim). + +## Deprecation shape + +`UpdatePrices(...).start()` emits `DeprecationWarning` and routes into the singleton with its +config (first-wins); it stores the returned handle and `.stop()` closes it. The context +manager maps to start/close. Behavioural deltas on this deprecated path (e.g. two `.start()` +calls no longer raise; mismatched config warns instead of preempting) are acceptable for a +deprecated surface at 0.x. + +## Status / open questions for review + +- Confirm with Samuel **why** `stop()` reverts to the bundled snapshot — Alex flagged this as + worth understanding before we lock in revert-on-close. +- Confirm Alex is bought into deleting the class (his huddle scope was "tweak `start`"), this + is a larger break — version bump to 0.1.0. diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index f4195ddd..37a94126 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -4,6 +4,7 @@ import logging import os import threading +import warnings from dataclasses import dataclass, field from time import time @@ -22,15 +23,25 @@ logger = logging.getLogger('genai-prices') DEFAULT_UPDATE_URL = 'https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json' -# This module-level state would not survive os.fork() on its own (threads die with the parent, -# and locks can be inherited in a held state, e.g. under gunicorn with preload_app=True). Fork -# hooks registered lazily in _register_fork_hooks() keep it consistent: the global lock is held -# across the fork and reinitialized in the child, and a running updater is restarted in place - -# see _fork_after_in_child. -_global_update_prices: UpdatePrices | None = None -_managed_update_prices: UpdatePrices | None = None -_managed_update_prices_ref_count = 0 -_global_update_prices_lock = threading.RLock() +DEFAULT_UPDATE_INTERVAL = 3600.0 + + +def _default_request_timeout() -> httpx2.Timeout: + return httpx2.Timeout(timeout=10, connect=5) + + +# There is exactly one price snapshot per process (data_snapshot.set_custom_snapshot is a global), +# so "updating prices" is inherently a singleton activity. The whole updater is therefore a single +# process-wide, ref-counted instance guarded by `_lock`: every caller of update_prices_in_background() +# gets an independent handle on the shared updater, and the updater stops when the last handle closes. +# +# This module-level state would not survive os.fork() on its own (threads die with the parent, and +# locks can be inherited in a held state, e.g. under gunicorn with preload_app=True). Fork hooks +# registered lazily in _register_fork_hooks() keep it consistent: the lock is held across the fork and +# reinitialized in the child, and a running updater is restarted in place - see _fork_after_in_child. +_updater: _BackgroundUpdater | None = None +_ref_count = 0 +_lock = threading.RLock() _fork_hooks_registered = False @@ -38,8 +49,8 @@ def _register_fork_hooks() -> None: """Keep the updater working across os.fork() (e.g. gunicorn with preload_app=True). - Called under `_global_update_prices_lock` from `_start_thread`, so registration happens at - most once, and only in processes that actually start an updater. + Called under `_lock` from `_BackgroundUpdater._start_thread`, so registration happens at most + once, and only in processes that actually start an updater. """ global _fork_hooks_registered if _fork_hooks_registered or not hasattr(os, 'register_at_fork'): @@ -51,30 +62,29 @@ def _register_fork_hooks() -> None: def _fork_before() -> None: # Hold the lock across the fork so the child inherits consistent bookkeeping rather than a # torn, mid-mutation state. - _global_update_prices_lock.acquire() + _lock.acquire() def _fork_after_in_parent() -> None: - _global_update_prices_lock.release() + _lock.release() def _fork_after_in_child() -> None: - global _global_update_prices_lock, _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count + global _lock, _updater, _ref_count # The child inherits the lock acquired in _fork_before; replace it with a fresh one rather # than releasing the inherited object. - _global_update_prices_lock = threading.RLock() - updater = _global_update_prices + _lock = threading.RLock() + updater = _updater if updater is None: return try: # The parent's updater thread does not survive the fork; restart it on the same instance, - # preserving identity so existing handles and the managed bookkeeping remain valid. + # preserving identity so existing handles remain valid. updater._revive_after_fork() # pyright: ignore[reportPrivateUsage] except Exception: - _global_update_prices = None - _managed_update_prices = None - _managed_update_prices_ref_count = 0 + _updater = None + _ref_count = 0 data_snapshot.set_custom_snapshot(None) logger.warning('Failed to restart the genai-prices background updater after fork', exc_info=True) @@ -85,10 +95,9 @@ def _fork_after_in_child() -> None: def _join_stopped_updater_thread(thread: threading.Thread | None) -> None: """Give a stopped updater thread a short grace period to exit, then abandon it. - Called outside `_global_update_prices_lock` so a thread blocked on an in-flight fetch never - stalls other updater API calls. Abandonment is safe: the thread is a daemon, it exits as soon - as the fetch completes, and a stopped updater can never install its result (see - `UpdatePrices._install_snapshot`). + Called outside `_lock` so a thread blocked on an in-flight fetch never stalls other updater + API calls. Abandonment is safe: the thread is a daemon, it exits as soon as the fetch + completes, and a stopped updater can never install its result (see `_install_snapshot`). """ if thread is None: return @@ -102,95 +111,101 @@ def _join_stopped_updater_thread(thread: threading.Thread | None) -> None: ) -def wait_prices_updated_sync(timeout: float | None = None) -> bool: - """Synchronously wait for prices to be updated. +def update_prices_in_background( + *, + url: str = DEFAULT_UPDATE_URL, + update_interval: float = DEFAULT_UPDATE_INTERVAL, + request_timeout: httpx2.Timeout | None = None, +) -> UpdatePricesHandle: + """Update prices in the background using a shared, process-wide daemon thread. - Never raises: if the update attempt failed, this returns False — the error is logged on the - `genai-prices` logger, and for a manually started `UpdatePrices` it is also re-raised from - `UpdatePrices.wait()` and `UpdatePrices.stop()`. + The first call starts the shared updater with the given configuration; later calls reuse it + and return independent handles. The updater is stopped when the last handle is closed, at + which point prices revert to the data bundled with the installed package. - Args: - timeout: The maximum time to wait for prices to be updated. Defaults to None which waits - indefinitely. + Configuration is first-wins: the first caller's `url`, `update_interval` and `request_timeout` + apply for the lifetime of the shared updater. A later caller passing different configuration + gets a warning and a handle on the already-running updater - its configuration is ignored. App + authors who need a custom URL should call this early in startup, before any library does. + + This function does not wait for the download: until the first fetch completes, price + calculations keep using the bundled data, and prices computed before then are not recalculated + once fresh data lands. Use `wait_prices_updated_sync` or `wait_prices_updated_async` if you + need fresh prices before calculating. Returns: - True if prices were updated, False otherwise (including when the update failed). + A handle that stops the shared background updater when all handles have been closed. """ - with _global_update_prices_lock: - update_prices = _global_update_prices + global _updater, _ref_count + + if request_timeout is None: + request_timeout = _default_request_timeout() + + with _lock: + if _updater is not None: + if _updater.url != url or _updater.update_interval != update_interval: + logger.warning( + 'A genai-prices background updater is already running with url=%r, ' + 'update_interval=%r; ignoring the new url=%r, update_interval=%r and returning ' + 'a handle on the existing updater. Configure the updater before any other ' + 'caller starts it if you need custom settings.', + _updater.url, + _updater.update_interval, + url, + update_interval, + ) + _ref_count += 1 + return UpdatePricesHandle(_updater) - if update_prices is not None: - return update_prices._wait_updated(timeout) # pyright: ignore[reportPrivateUsage] - return False + updater = _BackgroundUpdater(url=url, update_interval=update_interval, request_timeout=request_timeout) + _updater = updater + _ref_count = 1 + try: + updater._start_thread() # pyright: ignore[reportPrivateUsage] + except Exception: + _updater = None + _ref_count = 0 + raise + return UpdatePricesHandle(updater) -async def wait_prices_updated_async(timeout: float | None = None) -> bool: - """Asynchronously wait for prices to be updated. - Never raises: if the update attempt failed, this returns False — the error is logged on the - `genai-prices` logger, and for a manually started `UpdatePrices` it is also re-raised from - `UpdatePrices.wait()` and `UpdatePrices.stop()`. +def wait_prices_updated_sync(timeout: float | None = None) -> bool: + """Synchronously wait for prices to be updated by the shared background updater. + + Never raises: if the update attempt failed, this returns False - the error is logged on the + `genai-prices` logger. Args: - timeout: The maximum time to wait for prices to be updated. Defaults to None which waits indefinitely. + timeout: The maximum time to wait for prices to be updated. Defaults to None which waits + indefinitely. Returns: - True if prices were updated, False otherwise (including when the update failed). + True if prices were updated, False otherwise (including when the update failed, or when no + updater is running). """ - return await asyncio.to_thread(wait_prices_updated_sync, timeout) + with _lock: + updater = _updater + if updater is not None: + return updater._wait_updated(timeout) # pyright: ignore[reportPrivateUsage] + return False -def update_prices_in_background() -> UpdatePricesHandle: - """Update prices in the background using a shared, process-wide daemon thread. - The first call starts the shared updater; later calls reuse it and return independent handles. - The updater is stopped when the last handle is closed, at which point prices revert to the - data bundled with the installed package. +async def wait_prices_updated_async(timeout: float | None = None) -> bool: + """Asynchronously wait for prices to be updated by the shared background updater. - This function does not wait for the download: until the first fetch completes, price - calculations keep using the bundled data, and prices computed before then are not recalculated - once fresh data lands. Use `wait_prices_updated_sync` or `wait_prices_updated_async` if you - need fresh prices before calculating. + Never raises: if the update attempt failed, this returns False - the error is logged on the + `genai-prices` logger. - A manually started `UpdatePrices` always takes precedence over the shared updater. If one is - already running, no second updater is started and the returned handle does nothing on close — - prices are already being kept up to date. If one is started later, the shared updater is - stopped and existing handles become inert. Either way, an inert handle stays inert: if the - manual updater is later stopped, background updates stop with it, and a new handle must be - acquired to restart them. + Args: + timeout: The maximum time to wait for prices to be updated. Defaults to None which waits indefinitely. Returns: - A handle that stops the shared background updater when all handles have been closed. + True if prices were updated, False otherwise (including when the update failed, or when no + updater is running). """ - global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count - - with _global_update_prices_lock: - if _managed_update_prices is not None: - _managed_update_prices_ref_count += 1 - return UpdatePricesHandle(_managed_update_prices) - - if _global_update_prices is not None: - # A manually started UpdatePrices is already keeping prices fresh; don't start a - # second updater and don't claim the manual one — its owner controls its lifetime. - logger.info( - 'A manually started UpdatePrices is already running; update_prices_in_background() ' - 'is returning an inert handle and not starting a shared updater.' - ) - return UpdatePricesHandle() - - update_prices = UpdatePrices() - _global_update_prices = update_prices - _managed_update_prices = update_prices - _managed_update_prices_ref_count = 1 - try: - update_prices._start_thread() # pyright: ignore[reportPrivateUsage] - except Exception: - _global_update_prices = None - _managed_update_prices = None - _managed_update_prices_ref_count = 0 - raise - - return UpdatePricesHandle(update_prices) + return await asyncio.to_thread(wait_prices_updated_sync, timeout) @dataclass @@ -203,9 +218,11 @@ class UpdatePricesHandle: Do not copy a handle (`copy.copy`, `dataclasses.replace`, pickling): each handle represents exactly one claim, so closing a copy releases the original's claim and can stop the shared updater while other consumers still hold open handles. + + Can be used as a context manager; exiting the block closes the handle. """ - _update_prices: UpdatePrices | None = None + _update_prices: _BackgroundUpdater | None = None _closed: bool = field(default=False, init=False) def close(self): @@ -218,24 +235,23 @@ def close(self): flight the daemon thread is given a short grace period to exit before being abandoned with a warning log - a stopped updater can never install its result afterwards. """ - global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count + global _updater, _ref_count - with _global_update_prices_lock: + with _lock: if self._closed: return self._closed = True - if self._update_prices is None or _managed_update_prices is not self._update_prices: + if self._update_prices is None or _updater is not self._update_prices: return - _managed_update_prices_ref_count -= 1 - if _managed_update_prices_ref_count > 0: + _ref_count -= 1 + if _ref_count > 0: return update_prices = self._update_prices - _global_update_prices = None - _managed_update_prices = None - _managed_update_prices_ref_count = 0 + _updater = None + _ref_count = 0 thread = update_prices._stop_and_detach() # pyright: ignore[reportPrivateUsage] # Join outside the lock so a thread blocked on an in-flight fetch never stalls other @@ -246,85 +262,29 @@ def close(self): update_prices._background_exc = None # pyright: ignore[reportPrivateUsage] logger.error('Error from genai-prices background updater while closing', exc_info=exc) + def __enter__(self) -> UpdatePricesHandle: + return self -@dataclass -class UpdatePrices: - """Update prices in the background using a daemon thread. + def __exit__(self, *_args: object): + self.close() - Can be used either as a context manager or as a simple class, where you'll need to call start() and stop() manually. - """ - update_interval: float = 3600 - """How often to update prices in seconds.""" - url: str = DEFAULT_UPDATE_URL - """The URL to fetch prices from.""" - request_timeout: httpx2.Timeout = field(default_factory=lambda: httpx2.Timeout(timeout=10, connect=5)) - """The timeout for HTTP requests.""" +@dataclass +class _BackgroundUpdater: + """The single process-wide updater. Internal: consumers interact via `UpdatePricesHandle`.""" + + url: str + update_interval: float + request_timeout: httpx2.Timeout _stop_event: threading.Event = field(default_factory=threading.Event) _prices_updated: threading.Event = field(default_factory=threading.Event) - _snapshot_lock: threading.Lock = field(default_factory=threading.Lock, init=False) - _thread: threading.Thread | None = field(default=None, init=False) - _background_exc: Exception | None = field(default=None, init=False) - _update_succeeded: bool = field(default=False, init=False) - - def start(self, *, wait: bool | float = False): - """Start the background task. - - If the shared background updater started by `update_prices_in_background` is running, it is - stopped and this instance takes over: a manually started `UpdatePrices` always takes - precedence, and existing handles on the shared updater become inert. Prices briefly revert - to the bundled data until this instance's first fetch completes (pass `wait` to block until - then). Starting a second manually created `UpdatePrices` still raises `RuntimeError`. - - Args: - wait: Whether to wait for the prices to be updated before returning, if an int is passed - wait for that many seconds, if `True` wait for 30 seconds. - """ - global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count - - taken_over: UpdatePrices | None = None - drained_thread: threading.Thread | None = None - with _global_update_prices_lock: - if self._thread is not None: - raise RuntimeError('UpdatePrices background task already started') - - if _global_update_prices is not None: - if _global_update_prices is not _managed_update_prices: - raise RuntimeError( - 'UpdatePrices global task already started, only one UpdatePrices can be active at a time' - ) - - taken_over = _global_update_prices - logger.info( - 'Stopping the shared background updater started via update_prices_in_background(); ' - 'its open handles (e.g. held by libraries such as logfire) are now inert and this ' - 'manually started UpdatePrices takes over.' - ) - _global_update_prices = None - _managed_update_prices = None - _managed_update_prices_ref_count = 0 - drained_thread = taken_over._stop_and_detach() - - _global_update_prices = self - try: - self._start_thread() - except Exception: - _global_update_prices = None - raise - - if taken_over is not None: - _join_stopped_updater_thread(drained_thread) - exc = taken_over._background_exc - if exc: - taken_over._background_exc = None - logger.exception('Error from genai-prices background updater while taking over', exc_info=exc) - - if wait: - self.wait(timeout=30 if wait is True else wait) + _thread: threading.Thread | None = field(default=None) + _background_exc: Exception | None = field(default=None) + _update_succeeded: bool = field(default=False) def _start_thread(self) -> None: if self._thread is not None: - raise RuntimeError('UpdatePrices background task already started') + raise RuntimeError('genai-prices background task already started') _register_fork_hooks() self._prices_updated.clear() @@ -334,14 +294,14 @@ def _start_thread(self) -> None: self._thread = threading.Thread(target=self._background_task, daemon=True, name='genai_prices:update') self._thread.start() - def wait(self, timeout: float | None = None) -> bool: - """Wait for the prices to be updated in the background task. - - Raises the stored exception if the update attempt failed. + def _wait_updated(self, timeout: float | None) -> bool: + # Never raises and does not consume the stored background exception: failures are already + # logged by the background task, and the exception stays stored for handle close to surface. + return self._prices_updated.wait(timeout=timeout) and self._update_succeeded - Args: - timeout: The maximum time to wait for the prices to be updated in seconds. - """ + def _wait_raising(self, timeout: float | None) -> bool: + # Like _wait_updated, but raises (and consumes) the stored background exception - the + # historical UpdatePrices.wait() behaviour. prices_updated = self._prices_updated.wait(timeout=timeout) exc = self._background_exc if exc: @@ -349,76 +309,30 @@ def wait(self, timeout: float | None = None) -> bool: raise exc return prices_updated and self._update_succeeded - def _wait_updated(self, timeout: float | None) -> bool: - # Unlike wait(), never raises and does not consume the stored background exception: - # failures are already logged by the background task, and the exception stays stored for - # the updater's owner to surface via wait()/stop()/close(). - return self._prices_updated.wait(timeout=timeout) and self._update_succeeded - - def stop(self): - """Stop the background task. - - A no-op if this instance was never started or has already been stopped; it does not - affect any other updater that happens to be running. - - Prices revert to the bundled data immediately. The call then waits for the background - thread to exit (bounded by the request timeout if a fetch is in flight) without blocking - other updater API calls, and re-raises the stored background exception, if any. - """ - global _global_update_prices, _managed_update_prices, _managed_update_prices_ref_count - - with _global_update_prices_lock: - if self._thread is None: - return - - if _global_update_prices is self: - _global_update_prices = None - if _managed_update_prices is self: - _managed_update_prices = None - _managed_update_prices_ref_count = 0 - thread = self._stop_and_detach() - - # Join outside the global lock so concurrent updater API calls don't queue behind an - # in-flight fetch; the snapshot is already reverted and re-publication fenced off. - if thread is not None: - thread.join() - if self._background_exc: - exc = self._background_exc - self._background_exc = None - raise exc - def _revive_after_fork(self) -> None: # Threads do not survive fork, and the inherited events/locks may have been captured in an # arbitrary state (e.g. mid-install); recreate them and restart the background thread on - # this same instance. + # this same instance. The snapshot is fenced by the module `_lock`, which is itself + # replaced with a fresh one in `_fork_after_in_child`. self._thread = None self._stop_event = threading.Event() self._prices_updated = threading.Event() - self._snapshot_lock = threading.Lock() self._start_thread() def _stop_and_detach(self) -> threading.Thread | None: """Signal the background thread to stop and revert prices to the bundled data. - Does not join; returns the (possibly still draining) thread so the caller can wait on it - outside `_global_update_prices_lock`. Setting the stop event before clearing the snapshot, - both ordered against `_install_snapshot` via `_snapshot_lock`, guarantees an in-flight - fetch can finish but can never install its result afterwards. + Called with the module `_lock` held. Does not join; returns the (possibly still draining) + thread so the caller can wait on it after releasing `_lock`. Setting the stop event before + clearing the snapshot, both ordered against `_install_snapshot` via `_lock`, guarantees an + in-flight fetch can finish but can never install its result afterwards. """ thread = self._thread self._thread = None self._stop_event.set() - with self._snapshot_lock: - data_snapshot.set_custom_snapshot(None) + data_snapshot.set_custom_snapshot(None) return thread - def __enter__(self): - self.start() - return self - - def __exit__(self, *_args: object): - self.stop() - def _background_task(self) -> None: logger.info('Starting genai-prices background task') try: @@ -452,9 +366,9 @@ def _update_prices(self): def _install_snapshot(self, snapshot: data_snapshot.DataSnapshot | None) -> None: # Fencing check: never publish after stop. The stop path sets _stop_event and then clears - # the snapshot under the same lock (see _stop_and_detach), so re-checking the event here - # makes publish-after-stop impossible rather than merely unlikely. - with self._snapshot_lock: + # the snapshot under `_lock` (see _stop_and_detach), so taking the same lock here and + # re-checking the event makes publish-after-stop impossible rather than merely unlikely. + with _lock: if self._stop_event.is_set(): logger.info('genai-prices updater was stopped during the fetch; discarding the fetched snapshot') return @@ -467,3 +381,73 @@ def fetch(self) -> data_snapshot.DataSnapshot | None: r = httpx2.get(self.url, timeout=self.request_timeout) r.raise_for_status() return data_snapshot.DataSnapshot(data.providers_schema.validate_json(r.content), from_auto_update=True) + + +@dataclass +class UpdatePrices: + """Deprecated. Use `update_prices_in_background()` instead. + + Retained as a thin shim over the shared, process-wide updater so existing code keeps working. + It will be removed in a future release. + + Behavioural notes vs. the historical class: there is now a single shared updater, so a second + `start()` no longer raises and starting an instance no longer takes over (and stops) an updater + that another caller already started - configuration is first-wins. See `update_prices_in_background`. + """ + + update_interval: float = DEFAULT_UPDATE_INTERVAL + """How often to update prices in seconds.""" + url: str = DEFAULT_UPDATE_URL + """The URL to fetch prices from.""" + request_timeout: httpx2.Timeout = field(default_factory=_default_request_timeout) + """The timeout for HTTP requests.""" + _handle: UpdatePricesHandle | None = field(default=None, init=False) + + def start(self, *, wait: bool | float = False): + """Start the shared background updater (deprecated; use `update_prices_in_background()`).""" + warnings.warn( + 'UpdatePrices is deprecated; use update_prices_in_background() instead.', + DeprecationWarning, + stacklevel=2, + ) + if self._handle is not None and not self._handle._closed: # pyright: ignore[reportPrivateUsage] + raise RuntimeError('UpdatePrices background task already started') + + self._handle = update_prices_in_background( + url=self.url, update_interval=self.update_interval, request_timeout=self.request_timeout + ) + if wait: + self.wait(timeout=30 if wait is True else wait) + + def wait(self, timeout: float | None = None) -> bool: + """Wait for the prices to be updated by the shared background updater. + + Raises the stored exception if the update attempt failed. + """ + updater = self._handle._update_prices if self._handle is not None else None # pyright: ignore[reportPrivateUsage] + if updater is None: + return False + return updater._wait_raising(timeout) # pyright: ignore[reportPrivateUsage] + + def fetch(self) -> data_snapshot.DataSnapshot | None: + """Fetch the latest provider data from the configured URL (does not start a background task).""" + return _BackgroundUpdater( + url=self.url, update_interval=self.update_interval, request_timeout=self.request_timeout + ).fetch() + + def stop(self): + """Release this instance's claim on the shared updater (deprecated). + + Stops the shared updater only if this was the last open claim; under the shared model a + claim held by another caller keeps it running. + """ + if self._handle is not None: + self._handle.close() + self._handle = None + + def __enter__(self): + self.start() + return self + + def __exit__(self, *_args: object): + self.stop() diff --git a/tests/test_custom_prices.py b/tests/test_custom_prices.py index 8b6da9e8..717247a4 100644 --- a/tests/test_custom_prices.py +++ b/tests/test_custom_prices.py @@ -9,7 +9,27 @@ from genai_prices import Usage, calc_price, data, types from genai_prices.data_snapshot import DataSnapshot, set_custom_snapshot -from genai_prices.update_prices import UpdatePrices + + +class _CustomSnapshotSource: + """Test helper that installs a custom snapshot for the duration of a `with` block. + + Replaces subclassing the (deprecated) UpdatePrices: these tests only need a custom snapshot + active, not a background updater thread. Subclasses implement `fetch()`. + """ + + def fetch(self) -> DataSnapshot | None: + raise NotImplementedError + + def __enter__(self) -> _CustomSnapshotSource: + set_custom_snapshot(self.fetch()) + return self + + def wait(self, _timeout: float | None = None) -> bool: + return True + + def __exit__(self, *_args: object) -> None: + set_custom_snapshot(None) @dataclass @@ -23,7 +43,7 @@ def calc_price(self, usage: types.AbstractUsage) -> types.CalcPrice: return price -class AltUpdatePrices(UpdatePrices): +class AltUpdatePrices(_CustomSnapshotSource): def fetch(self) -> DataSnapshot | None: custom_providers = [ types.Provider( @@ -53,7 +73,7 @@ def fetch(self) -> DataSnapshot | None: return DataSnapshot(providers=custom_providers, from_auto_update=False) -class ExtraUpdatePrices(UpdatePrices): +class ExtraUpdatePrices(_CustomSnapshotSource): def fetch(self) -> DataSnapshot | None: providers = deepcopy(data.providers) openai = next(provider for provider in providers if provider.id == 'openai') diff --git a/tests/test_tiered_pricing.py b/tests/test_tiered_pricing.py index ae7329ef..e8fca34b 100644 --- a/tests/test_tiered_pricing.py +++ b/tests/test_tiered_pricing.py @@ -5,17 +5,30 @@ import pytest from inline_snapshot import snapshot -from genai_prices import Usage, calc_price, types +from genai_prices import Usage, calc_price, data_snapshot, types from genai_prices.data_snapshot import DataSnapshot -from genai_prices.update_prices import UpdatePrices pytestmark = pytest.mark.anyio -class MultiTierUpdatePrices(UpdatePrices): - """Custom UpdatePrices that injects a multi-tier pricing model for testing.""" +class MultiTierUpdatePrices: + """Test helper that installs a multi-tier pricing snapshot for the duration of a `with` block. - def fetch(self) -> DataSnapshot | None: + These tests only need a custom snapshot to be active, not a real background updater thread, so + this sets the snapshot directly rather than going through the (deprecated) UpdatePrices class. + """ + + def __enter__(self) -> MultiTierUpdatePrices: + data_snapshot.set_custom_snapshot(self._build_snapshot()) + return self + + def wait(self, _timeout: float | None = None) -> bool: + return True + + def __exit__(self, *_args: object) -> None: + data_snapshot.set_custom_snapshot(None) + + def _build_snapshot(self) -> DataSnapshot: """Create a mock provider with a multi-tier pricing model. Pricing structure (threshold-based with different tiers for input vs output): diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 94d9b192..174a88c2 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -57,6 +57,7 @@ def test_update_prices_fetch_parses_provider_array(monkeypatch: pytest.MonkeyPat _mock_update_prices_get(monkeypatch, content) + # fetch() does not emit a deprecation warning (only start() does). snapshot = UpdatePrices(url='https://example.test/prices.json').fetch() assert snapshot is not None @@ -66,11 +67,11 @@ def test_update_prices_fetch_parses_provider_array(monkeypatch: pytest.MonkeyPat assert model.id == 'gpt-4o-mini' -def test_update_prices_wait_on_start(monkeypatch: pytest.MonkeyPatch): +def test_background_update_and_calc_price(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) assert data_snapshot._custom_snapshot is None - with UpdatePrices() as update_prices: - update_prices.wait() + with update_prices_in_background(): + assert wait_prices_updated_sync(timeout=5) assert data_snapshot._custom_snapshot is not None price = calc_price(Usage(input_tokens=1000, output_tokens=100), model_ref='gpt-4o', provider_id='openai') assert price.input_price == snapshot(Decimal('0.0025')) @@ -78,128 +79,37 @@ def test_update_prices_wait_on_start(monkeypatch: pytest.MonkeyPatch): assert price.total_price == snapshot(Decimal('0.0035')) assert price.provider.id == snapshot('openai') assert price.auto_update_timestamp is not None - - -def test_wait_prices_updated_sync(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) assert data_snapshot._custom_snapshot is None - with UpdatePrices(): - wait_prices_updated_sync() - assert data_snapshot._custom_snapshot is not None - price = calc_price(Usage(input_tokens=1000, output_tokens=100), model_ref='gpt-4o', provider_id='openai') - assert price.input_price == snapshot(Decimal('0.0025')) - assert price.output_price == snapshot(Decimal('0.001')) - assert price.total_price == snapshot(Decimal('0.0035')) - assert price.provider.id == snapshot('openai') - assert price.auto_update_timestamp is not None async def test_wait_prices_updated_async(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) assert data_snapshot._custom_snapshot is None - with UpdatePrices(): - await wait_prices_updated_async() + with update_prices_in_background(): + assert await wait_prices_updated_async(timeout=5) assert data_snapshot._custom_snapshot is not None - price = calc_price(Usage(input_tokens=1000, output_tokens=100), model_ref='gpt-4o', provider_id='openai') - assert price.input_price == snapshot(Decimal('0.0025')) - assert price.output_price == snapshot(Decimal('0.001')) - assert price.total_price == snapshot(Decimal('0.0035')) - assert price.provider.id == snapshot('openai') - assert price.auto_update_timestamp is not None - - -def test_update_prices_stop_clears_snapshot_after_in_flight_fetch(monkeypatch: pytest.MonkeyPatch) -> None: - fetch_started = threading.Event() - allow_fetch_return = threading.Event() - stop_errors: list[BaseException] = [] - - class Response: - content = PROVIDER_ARRAY_PAYLOAD - - def raise_for_status(self) -> None: - pass - - def fake_get(url: str, timeout: httpx2.Timeout) -> Response: - assert url == 'https://example.test/prices.json' - assert timeout is not None - fetch_started.set() - assert allow_fetch_return.wait(timeout=5) - return Response() + assert data_snapshot._custom_snapshot is None - def stop_update_prices(update_prices: UpdatePrices) -> None: - try: - update_prices.stop() - except BaseException as exc: - stop_errors.append(exc) - monkeypatch.setattr(httpx2, 'get', fake_get) - update_prices = UpdatePrices(url='https://example.test/prices.json', update_interval=3600) - update_prices.start() +def test_first_wins_config(monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture): + _mock_update_prices_get(monkeypatch) + handle_1 = update_prices_in_background(url='https://example.test/prices.json') try: - assert fetch_started.wait(timeout=5) - - stop_thread = threading.Thread(target=stop_update_prices, args=(update_prices,)) - stop_thread.start() - assert update_prices._stop_event.wait(timeout=5) - allow_fetch_return.set() - stop_thread.join(timeout=5) - - assert not stop_thread.is_alive() - if stop_errors: - raise stop_errors[0] - assert data_snapshot._custom_snapshot is None + with caplog.at_level('WARNING', logger='genai-prices'): + handle_2 = update_prices_in_background(url=DEFAULT_UPDATE_URL, update_interval=1) + try: + # The first caller's configuration wins; the second joins the running updater. + assert handle_1 is not handle_2 + assert update_prices_module._updater is not None + assert update_prices_module._updater.url == 'https://example.test/prices.json' + assert update_prices_module._updater.update_interval == 3600 + assert any('already running' in record.message for record in caplog.records) + assert wait_prices_updated_sync(timeout=5) + finally: + handle_2.close() finally: - allow_fetch_return.set() - update_prices.stop() + handle_1.close() data_snapshot.set_custom_snapshot(None) - - -@pytest.mark.default_cassette('fail.yaml') -@pytest.mark.vcr() -def test_update_prices_failed(): - assert data_snapshot._custom_snapshot is None - with UpdatePrices(url='https://demo-endpoints.pydantic.workers.dev/bin?status=404') as update_prices: - with pytest.raises(httpx2.HTTPStatusError): - update_prices.wait() - assert data_snapshot._custom_snapshot is None - - -@pytest.mark.default_cassette('fail.yaml') -@pytest.mark.vcr() -def test_update_prices_failed_stop(): - assert data_snapshot._custom_snapshot is None - update_prices = UpdatePrices(url='https://demo-endpoints.pydantic.workers.dev/bin?status=404') - update_prices.start() - with pytest.raises(httpx2.HTTPStatusError): - update_prices.stop() - assert data_snapshot._custom_snapshot is None - - -def test_update_prices_multiple(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - with UpdatePrices(): - with pytest.raises( - RuntimeError, - match='UpdatePrices global task already started, only one UpdatePrices can be active at a time', - ): - with UpdatePrices(): - pass - - -def test_update_prices_in_background_inert_when_manual_updater_running( - monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture -): - _mock_update_prices_get(monkeypatch) - with UpdatePrices(): - with caplog.at_level('INFO', logger='genai-prices'): - handle = update_prices_in_background() - assert any('returning an inert handle' in record.message for record in caplog.records) - assert wait_prices_updated_sync(timeout=5) - assert data_snapshot._custom_snapshot is not None - - handle.close() - assert wait_prices_updated_sync(timeout=0) - assert data_snapshot._custom_snapshot is not None assert data_snapshot._custom_snapshot is None @@ -261,7 +171,7 @@ def test_update_prices_in_background_ref_count(monkeypatch: pytest.MonkeyPatch): assert wait_prices_updated_sync(timeout=0) assert data_snapshot._custom_snapshot is not None - handle_1.close() + handle_1.close() # idempotent assert wait_prices_updated_sync(timeout=0) assert data_snapshot._custom_snapshot is not None @@ -274,62 +184,46 @@ def test_update_prices_in_background_ref_count(monkeypatch: pytest.MonkeyPatch): data_snapshot.set_custom_snapshot(None) -def test_manual_start_takes_over_shared_updater(monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture): +def test_update_prices_handle_directly_constructed_is_inert(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) handle = update_prices_in_background() try: assert wait_prices_updated_sync(timeout=5) - with caplog.at_level('INFO', logger='genai-prices'): - with UpdatePrices() as manual: - manual.wait(timeout=5) - assert data_snapshot._custom_snapshot is not None - assert update_prices_module._global_update_prices is manual - assert update_prices_module._managed_update_prices is None - # the pre-takeover handle is inert: closing it does not stop the manual updater - handle.close() - assert data_snapshot._custom_snapshot is not None - assert update_prices_module._global_update_prices is manual - assert any('takes over' in record.message for record in caplog.records) - assert data_snapshot._custom_snapshot is None - assert not [t for t in threading.enumerate() if t.name == 'genai_prices:update'] + UpdatePricesHandle().close() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None finally: handle.close() data_snapshot.set_custom_snapshot(None) + assert data_snapshot._custom_snapshot is None -def test_background_updater_restarts_after_manual_takeover_stops(monkeypatch: pytest.MonkeyPatch): +def test_stale_handle_close_does_not_affect_new_shared_updater(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) handle_1 = update_prices_in_background() - try: - assert wait_prices_updated_sync(timeout=5) - with UpdatePrices(): - pass - assert data_snapshot._custom_snapshot is None - - handle_2 = update_prices_in_background() - try: - assert wait_prices_updated_sync(timeout=5) - assert data_snapshot._custom_snapshot is not None - finally: - handle_2.close() - assert data_snapshot._custom_snapshot is None - finally: - handle_1.close() - data_snapshot.set_custom_snapshot(None) + assert wait_prices_updated_sync(timeout=5) + stale_updater = update_prices_module._updater + assert stale_updater is not None + # Fully release the first updater, then start a fresh one. + handle_1.close() + assert data_snapshot._custom_snapshot is None -def test_update_prices_handle_directly_constructed_is_inert(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - handle = update_prices_in_background() + handle_2 = update_prices_in_background() try: assert wait_prices_updated_sync(timeout=5) + new_updater = update_prices_module._updater + assert new_updater is not None and new_updater is not stale_updater - UpdatePricesHandle().close() + # A handle bound to the now-defunct first updater must not release handle_2's claim. + UpdatePricesHandle(stale_updater).close() + assert update_prices_module._updater is new_updater + assert update_prices_module._ref_count == 1 assert wait_prices_updated_sync(timeout=0) assert data_snapshot._custom_snapshot is not None finally: - handle.close() + handle_2.close() data_snapshot.set_custom_snapshot(None) assert data_snapshot._custom_snapshot is None @@ -345,7 +239,7 @@ def fake_get(*_args: object, **_kwargs: object) -> object: try: # Wait on the internal event rather than wait_prices_updated_sync, which would # consume the stored exception that close() is expected to log instead of raise. - updater = update_prices_module._managed_update_prices + updater = update_prices_module._updater assert updater is not None assert updater._prices_updated.wait(timeout=5) finally: @@ -376,45 +270,6 @@ def fake_get(*_args: object, **_kwargs: object) -> object: assert any('while closing' in record.message for record in caplog.records) -def test_stale_handle_close_does_not_affect_new_shared_updater(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - handle_1 = update_prices_in_background() - try: - assert wait_prices_updated_sync(timeout=5) - with UpdatePrices(): - pass - - handle_2 = update_prices_in_background() - try: - assert wait_prices_updated_sync(timeout=5) - new_updater = update_prices_module._managed_update_prices - assert new_updater is not None - - # handle_1 is bound to the pre-takeover updater: closing it while a different - # shared updater is live must not release handle_2's claim. - handle_1.close() - assert update_prices_module._managed_update_prices is new_updater - assert update_prices_module._managed_update_prices_ref_count == 1 - assert wait_prices_updated_sync(timeout=0) - assert data_snapshot._custom_snapshot is not None - finally: - handle_2.close() - assert data_snapshot._custom_snapshot is None - finally: - handle_1.close() - data_snapshot.set_custom_snapshot(None) - - -def test_stop_on_unstarted_instance_is_noop(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - with UpdatePrices() as update_prices: - assert update_prices.wait(timeout=5) - UpdatePrices().stop() - assert update_prices_module._global_update_prices is update_prices - assert data_snapshot._custom_snapshot is not None - assert data_snapshot._custom_snapshot is None - - @pytest.mark.skipif(not hasattr(os, 'fork'), reason='requires os.fork') def test_forked_child_restarts_shared_updater(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) @@ -430,7 +285,7 @@ def test_forked_child_restarts_shared_updater(monkeypatch: pytest.MonkeyPatch): wait_prices_updated_sync(timeout=5) and any(t.name == 'genai_prices:update' and t.is_alive() for t in threading.enumerate()) and data_snapshot._custom_snapshot is not None - and update_prices_module._global_update_prices is not None + and update_prices_module._updater is not None ) os._exit(0 if ok else 1) except BaseException: @@ -467,9 +322,8 @@ def worker() -> None: assert not [t for t in threads if t.is_alive()] assert not errors - assert update_prices_module._global_update_prices is None - assert update_prices_module._managed_update_prices is None - assert update_prices_module._managed_update_prices_ref_count == 0 + assert update_prices_module._updater is None + assert update_prices_module._ref_count == 0 assert not [t for t in threading.enumerate() if t.name == 'genai_prices:update'] assert data_snapshot._custom_snapshot is None @@ -481,3 +335,71 @@ def worker() -> None: finally: handle.close() data_snapshot.set_custom_snapshot(None) + + +# --- Deprecated UpdatePrices shim ------------------------------------------------------------ + + +def test_deprecated_update_prices_warns_and_routes_through_shared_updater(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + assert data_snapshot._custom_snapshot is None + with pytest.warns(DeprecationWarning, match='update_prices_in_background'): + with UpdatePrices() as update_prices: + assert update_prices.wait(timeout=5) + assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None + + +def test_deprecated_update_prices_multiple_instances_no_longer_raise(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + # Two distinct instances now share the one updater (first-wins) instead of raising. + with pytest.warns(DeprecationWarning): + with UpdatePrices(): + with UpdatePrices(): + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None + + +def test_deprecated_update_prices_same_instance_double_start_raises(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + update_prices = UpdatePrices() + with pytest.warns(DeprecationWarning): + update_prices.start() + try: + with pytest.warns(DeprecationWarning): + with pytest.raises(RuntimeError, match='already started'): + update_prices.start() + finally: + update_prices.stop() + data_snapshot.set_custom_snapshot(None) + + +@pytest.mark.default_cassette('fail.yaml') +@pytest.mark.vcr() +def test_deprecated_update_prices_wait_raises_on_failed_fetch(): + assert data_snapshot._custom_snapshot is None + update_prices = UpdatePrices(url='https://demo-endpoints.pydantic.workers.dev/bin?status=404') + with pytest.warns(DeprecationWarning): + update_prices.start() + try: + with pytest.raises(httpx2.HTTPStatusError): + update_prices.wait(timeout=5) + finally: + update_prices.stop() + assert data_snapshot._custom_snapshot is None + + +def test_deprecated_stop_on_unstarted_instance_is_noop(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + handle = update_prices_in_background() + try: + assert wait_prices_updated_sync(timeout=5) + # stop() on a never-started instance does not warn and does not touch the live updater. + UpdatePrices().stop() + assert update_prices_module._updater is not None + assert data_snapshot._custom_snapshot is not None + finally: + handle.close() + data_snapshot.set_custom_snapshot(None) + assert data_snapshot._custom_snapshot is None From a5296fa0d162c050c2b7dc9c87831709c7140cad Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Fri, 26 Jun 2026 19:32:23 +0530 Subject: [PATCH 14/17] Update README for the singleton background updater Lead with update_prices_in_background() as the primary, configurable, ref-counted entry point with first-wins config and library/app guidance; document UpdatePrices as deprecated. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/python/README.md | 94 +++++++++++++++------------------------ 1 file changed, 37 insertions(+), 57 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index 7a244891..3b60dfe4 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -87,84 +87,56 @@ price = extracted_usage.calc_price() print(price.total_price) ``` -### `UpdatePrices` +### Updating prices in the background -`UpdatePrices` can be used to periodically update the price data by downloading it from GitHub +`update_prices_in_background()` periodically updates the price data by downloading it from GitHub. Please note: - this functionality is explicitly opt-in - we download data directly from GitHub (`https://raw.githubusercontent.com/pydantic/genai-prices/refs/heads/main/prices/data.json`) so we don't and can't monitor requests or gather telemetry -At the time of writing, the `data.json` file -downloaded by `UpdatePrices` is around 26KB when compressed, so is generally very quick to download. - -By default `UpdatePrices` downloads price data immediately after it's started in the background, then every hour after that. - -Usage with `UpdatePrices` as as context manager: +At the time of writing, the `data.json` file downloaded is around 26KB when compressed, so is generally very +quick to download. By default the first fetch happens immediately in the background, then every hour after that. ```py -from genai_prices import UpdatePrices, Usage, calc_price +from genai_prices import update_prices_in_background -with UpdatePrices() as update_prices: - update_prices.wait() # optionally wait for prices to have updated - p = calc_price(Usage(input_tokens=123, output_tokens=456), 'gpt-5') - print(p) +handle = update_prices_in_background() +... +handle.close() # stop updating when you no longer need it ``` -Usage with `UpdatePrices` as a simple class: - -```py -from genai_prices import UpdatePrices, Usage, calc_price - -update_prices = UpdatePrices() -update_prices.start(wait=True) # start updating prices, optionally wait for prices to have updated -p = calc_price(Usage(input_tokens=123, output_tokens=456), 'gpt-5') -print(p) -update_prices.stop() # stop updating prices -``` +A single shared, process-wide updater backs this function, so it is safe to call from anywhere — including from +multiple libraries in the same process. The first call starts the updater; later calls reuse it and return +independent handles. The updater is stopped only when the **last** handle is closed, at which point prices revert +to the data bundled with the installed package. The handle is also a context manager (`with +update_prices_in_background() as handle: ...`). -Only one `UpdatePrices` instance can be running at a time. +**For libraries and integrations** (e.g. Logfire, Pydantic AI): call `update_prices_in_background()` with no +arguments, once, at startup. If several libraries do this, they share the one updater rather than spawning +duplicate threads, and each library's handle independently keeps it alive until closed. Leave configuration to +the application. -For libraries and integrations that want to opt into updating prices without creating duplicate background -threads, use `update_prices_in_background()`: +**For application authors** who need a custom URL or refresh interval: pass them, and call early in startup, +before the libraries initialize: ```py -from genai_prices import update_prices_in_background - -update_prices_handle = update_prices_in_background() -... -update_prices_handle.close() +update_prices_in_background(url='https://my-mirror.example/prices.json', update_interval=1800) ``` -The first call starts a shared process-wide updater with default settings (hourly refresh from the default URL — -the shared updater is not configurable; if you need a custom URL or interval, use `UpdatePrices` directly). Later -calls reuse the same updater and return independent handles. The updater is stopped when the last handle is -closed, at which point prices revert to the data bundled with the installed package. - -A manually started `UpdatePrices` always takes precedence over the shared updater, regardless of which started -first: - -- If an `UpdatePrices` instance has already been started manually, `update_prices_in_background()` does not start - a second updater and returns a handle that does nothing on close: prices are already being kept up to date, and - the manual updater's lifetime stays with whoever started it. -- If `UpdatePrices.start()` is called while the shared updater is running, the shared updater is stopped and the - manual instance takes over; existing handles become inert. Prices briefly revert to the bundled data until the - manual updater's first fetch completes — pass `wait` to `start()` to block until then. - -Either way, an inert handle stays inert: if the manual updater is later stopped, background updates stop with -it — call `update_prices_in_background()` again to start a new shared updater. Both precedence cases are logged -at `INFO` level on the `genai-prices` logger, which is the place to look if background updates ever stop -unexpectedly. +Configuration is **first-wins**: the first caller's `url`, `update_interval` and `request_timeout` apply for the +lifetime of the shared updater. A later caller passing different settings gets a handle on the already-running +updater and a warning logged on the `genai-prices` logger — its settings are ignored. Calling early ensures your +configuration is the one that takes effect. `UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged instead. Closing the last handle stops the updater and reverts prices to the bundled data immediately. If a fetch is in flight, the daemon thread is given a short grace period to exit and is otherwise abandoned with a warning log: it exits once the fetch completes, and its result is discarded — a stopped updater can never -install prices afterwards. (`UpdatePrices.stop()` instead waits for the thread to exit, so a manual stop can -block its caller for roughly the request timeout; neither blocks other updater API calls, and `calc_price` -is always unaffected.) A handle represents exactly one claim on the updater — don't copy one, as closing the -copy releases the original's claim too. +install prices afterwards. Other updater API calls are never blocked, and `calc_price` is always unaffected. A +handle represents exactly one claim on the updater — don't copy one, as closing the copy releases the original's +claim too. `update_prices_in_background()` does not wait for the download. Until the first fetch completes, `calc_price` keeps using the data bundled with the installed package, so prices for models released after that snapshot may be @@ -174,7 +146,7 @@ short-lived script), call `wait_prices_updated_sync()` / `wait_prices_updated_as handle — these never raise and return `False` if the update failed (the error is logged on the `genai-prices` logger), so your calculations simply fall back to the bundled data. -If you'd like to wait for prices to be updated without access to the `UpdatePrices` instance, you can use the `wait_prices_updated_sync` function: +You can wait for prices to be updated from anywhere with `wait_prices_updated_sync`: ```py from genai_prices import wait_prices_updated_sync @@ -183,7 +155,15 @@ wait_prices_updated_sync() ... ``` -Or it's async variant, `wait_prices_updated_async`. +Or its async variant, `wait_prices_updated_async`. + +#### Deprecated: the `UpdatePrices` class + +The `UpdatePrices` class (`UpdatePrices().start()` / `.stop()` / `with UpdatePrices():`) is **deprecated** and +emits a `DeprecationWarning`. It now routes through the same shared updater described above and will be removed in +a future release — use `update_prices_in_background()` instead. Note the behaviour change: because there is now a +single shared updater, starting a second `UpdatePrices` no longer raises and no longer takes over an updater +another caller already started; configuration is first-wins. ### CLI Usage From 41931331fb82182fa8099bd0af6c2ccfc7ee2454 Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Fri, 26 Jun 2026 19:44:27 +0530 Subject: [PATCH 15/17] Address review: timeout conflict warning, stop() re-raises, drop ADR - update_prices_in_background() now also warns when request_timeout differs from the running updater, matching the documented first-wins contract. - Restore the historical UpdatePrices.stop() behaviour of re-raising a stored background fetch error: split handle teardown into _release() (returns the exception) so close() logs it while the deprecated stop() raises it. - Remove docs/adr/0001 (internal decision aid, not repo material). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/adr/0001-singleton-background-updater.md | 116 ------------------ packages/python/genai_prices/update_prices.py | 50 +++++--- tests/test_update_prices.py | 13 ++ 3 files changed, 48 insertions(+), 131 deletions(-) delete mode 100644 docs/adr/0001-singleton-background-updater.md diff --git a/docs/adr/0001-singleton-background-updater.md b/docs/adr/0001-singleton-background-updater.md deleted file mode 100644 index 8c929e60..00000000 --- a/docs/adr/0001-singleton-background-updater.md +++ /dev/null @@ -1,116 +0,0 @@ -# ADR 0001 — Collapse the background updater into a process-global singleton - -- Status: Proposed (for review with Alex / Samuel) -- Date: 2026-06-26 -- Context: PR #404, follow-up to the 19 Jun huddle - -## The call - -Replace the instantiable `UpdatePrices` class + per-instance `start()`/`stop()` with a -single process-global, ref-counted updater fronted by one function: - -```python -handle = genai_prices.update_prices_in_background( - *, url=..., interval=3600, timeout=..., # all optional -) -... -handle.close() # or: with update_prices_in_background() as h: ... - -genai_prices.wait_for_update(timeout=...) # sync (role unchanged) -await genai_prices.wait_for_update_async(...) # async -``` - -`UpdatePrices` is **deprecated**, not removed: it stays importable as a thin shim that -warns and routes through the singleton. It is removed in a later release. - -## Why - -There is exactly **one** price snapshot per process — `data_snapshot.set_custom_snapshot()` -is a process-global. "Updating prices" is therefore inherently a singleton activity: two -"independent" updaters would both write the same global. - -The current public API contradicts that domain fact. It exposes an instantiable class with -per-instance `start()`/`stop()`/config/context-manager, while every instance secretly shares -one global snapshot. That mismatch — N instances pretending to be independent — is the root -cause of the complexity we maintain: - -- `RuntimeError` "only one can be active at a time" -- takeover / precedence logic (manual instance preempts the shared updater) -- the duplicated entry points (`update_prices_in_background()` _and_ `UpdatePrices().start()`) -- refcount living in module globals while ownership lives on instances - -None of that is essential to the problem; it is all tax on pretending instances are independent. -Folding `update_prices_in_background()` into `start()` (the huddle's lighter option) does not -remove the tax — it just relocates the unanswered question (what happens with mismatched config -across instances) from API names into behaviour. Deleting the class answers it by construction. - -## User-facing model - -"Call `update_prices_in_background()` from anywhere, as many times as you like; close your -handle when done." Libraries (logfire, pydantic-ai) call it config-less and never close. -App authors who want a custom URL pass it, early in startup. - -Prior art: the `logging` / threading-singleton pattern — a process-global resource you -reference, not instantiate. The "configure early" rule below is the same discipline -`logging.basicConfig` teaches. - -## Config-conflict rule: first-wins + warning - -Two callers passing _different_ config (the multi-URL case) is the only real ambiguity. - -- **First-wins (chosen):** first caller's config sticks; a later caller with mismatched - config gets a warning and a handle on the already-running updater. Config-less library - calls always just join — they never conflict. App authors who want custom config call - early, before libraries initialize. -- Last-wins / takeover — rejected: reintroduces the exact state machine we are deleting. -- Raise — rejected: a library starting first would crash the app author. - -This keeps the lifecycle a monotonic state machine: empty -> configured+running -> -(refcount hits 0) -> stopped. No backward transitions, no preemption. - -## Decisions parked deliberately - -### Revert-on-close: KEEP today's behaviour - -When the last handle closes, prices revert to the package-bundled data (not the -last-fetched snapshot). The "a fetch in flight at stop time can never publish afterward" -fencing invariant therefore **stays**. We considered leaving the fresh snapshot in place -(which would let us drop the fencing entirely) but chose to preserve current semantics for -now. Revisit if/when we want that simplification. - -### Fork support: separate, stacked PR - -The `os.register_at_fork` hooks (surviving gunicorn `preload_app=True`) are orthogonal and -land as a follow-up so this change can be reviewed on its own. Under the simpler singleton -core the fork story is also easier: one thread, one config, no takeover to revive. - -## What stays / goes internally - -- A single module `_lock` (`RLock`) — **stays, and is now the only lock.** Load-bearing for - refcount + thread-handle bookkeeping (two racing first-calls must not both spawn a thread). - NOTE for the huddle's "worst case is just redundant network calls": that undersells it — - without the lock, two racing starts leak an untracked daemon thread and corrupt the refcount, - which is a correctness bug. -- The separate `_snapshot_lock` — **gone.** Its fencing job (ordering the snapshot install in - the background thread against the stop/revert) is folded onto the same module `_lock`, since - both ultimately guard the one process-global snapshot. `set_custom_snapshot` is a bare global - assignment (no internal lock, no callback), so holding `_lock` across it cannot deadlock or - invert lock order. Result: one lock guarding a trivial state machine (a pointer, an int, the - snapshot). -- Takeover / precedence / `RuntimeError` "only one active" — **gone.** -- `UpdatePrices.start()/stop()`/context-manager — **gone** (live on only via the deprecated shim). - -## Deprecation shape - -`UpdatePrices(...).start()` emits `DeprecationWarning` and routes into the singleton with its -config (first-wins); it stores the returned handle and `.stop()` closes it. The context -manager maps to start/close. Behavioural deltas on this deprecated path (e.g. two `.start()` -calls no longer raise; mismatched config warns instead of preempting) are acceptable for a -deprecated surface at 0.x. - -## Status / open questions for review - -- Confirm with Samuel **why** `stop()` reverts to the bundled snapshot — Alex flagged this as - worth understanding before we lock in revert-on-close. -- Confirm Alex is bought into deleting the class (his huddle scope was "tweak `start`"), this - is a larger break — version bump to 0.1.0. diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 37a94126..a0fc9a30 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -143,16 +143,23 @@ def update_prices_in_background( with _lock: if _updater is not None: - if _updater.url != url or _updater.update_interval != update_interval: + if ( + _updater.url != url + or _updater.update_interval != update_interval + or _updater.request_timeout != request_timeout + ): logger.warning( - 'A genai-prices background updater is already running with url=%r, ' - 'update_interval=%r; ignoring the new url=%r, update_interval=%r and returning ' - 'a handle on the existing updater. Configure the updater before any other ' - 'caller starts it if you need custom settings.', + 'A genai-prices background updater is already running (url=%r, update_interval=%r, ' + 'request_timeout=%r); ignoring the different configuration passed to this call ' + '(url=%r, update_interval=%r, request_timeout=%r) and returning a handle on the ' + 'existing updater. Configure the updater before any other caller starts it if you ' + 'need custom settings.', _updater.url, _updater.update_interval, + _updater.request_timeout, url, update_interval, + request_timeout, ) _ref_count += 1 return UpdatePricesHandle(_updater) @@ -235,19 +242,29 @@ def close(self): flight the daemon thread is given a short grace period to exit before being abandoned with a warning log - a stopped updater can never install its result afterwards. """ + exc = self._release() + if exc is not None: + logger.error('Error from genai-prices background updater while closing', exc_info=exc) + + def _release(self) -> Exception | None: + """Release this handle's claim and tear down the updater if it was the last one. + + Returns the stored background exception (if any) for the caller to surface: the public + `close()` logs it, while the deprecated `UpdatePrices.stop()` re-raises it. Idempotent. + """ global _updater, _ref_count with _lock: if self._closed: - return + return None self._closed = True if self._update_prices is None or _updater is not self._update_prices: - return + return None _ref_count -= 1 if _ref_count > 0: - return + return None update_prices = self._update_prices _updater = None @@ -258,9 +275,8 @@ def close(self): # updater API calls; re-publication is already fenced off by _stop_and_detach(). _join_stopped_updater_thread(thread) exc = update_prices._background_exc # pyright: ignore[reportPrivateUsage] - if exc: - update_prices._background_exc = None # pyright: ignore[reportPrivateUsage] - logger.error('Error from genai-prices background updater while closing', exc_info=exc) + update_prices._background_exc = None # pyright: ignore[reportPrivateUsage] + return exc def __enter__(self) -> UpdatePricesHandle: return self @@ -439,11 +455,15 @@ def stop(self): """Release this instance's claim on the shared updater (deprecated). Stops the shared updater only if this was the last open claim; under the shared model a - claim held by another caller keeps it running. + claim held by another caller keeps it running. Re-raises a stored background fetch error, + preserving the historical `UpdatePrices.stop()` behaviour. """ - if self._handle is not None: - self._handle.close() - self._handle = None + if self._handle is None: + return + exc = self._handle._release() # pyright: ignore[reportPrivateUsage] + self._handle = None + if exc is not None: + raise exc def __enter__(self): self.start() diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 174a88c2..27197458 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -390,6 +390,19 @@ def test_deprecated_update_prices_wait_raises_on_failed_fetch(): assert data_snapshot._custom_snapshot is None +@pytest.mark.default_cassette('fail.yaml') +@pytest.mark.vcr() +def test_deprecated_update_prices_stop_raises_on_failed_fetch(): + # The deprecated stop() preserves the historical behaviour of re-raising a stored fetch error. + assert data_snapshot._custom_snapshot is None + update_prices = UpdatePrices(url='https://demo-endpoints.pydantic.workers.dev/bin?status=404') + with pytest.warns(DeprecationWarning): + update_prices.start() + with pytest.raises(httpx2.HTTPStatusError): + update_prices.stop() + assert data_snapshot._custom_snapshot is None + + def test_deprecated_stop_on_unstarted_instance_is_noop(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) handle = update_prices_in_background() From 189886bfdca20bc7c2f2b5da5a33aaca5f2a5d8d Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Mon, 29 Jun 2026 09:47:52 +0530 Subject: [PATCH 16/17] Make UpdatePrices the ref-counted lease; drop update_prices_in_background Per review (alexmojaki): don't add a second public surface. Collapse back onto UpdatePrices as the single API and make starting it a ref-counted claim on one shared, process-wide updater (first-wins config, warn on mismatch). Remove update_prices_in_background() and UpdatePricesHandle. UpdatePrices is no longer deprecated; it is the primary library API. As the library-shutdown-friendly surface, stop() now logs background fetch errors instead of raising (wait() still raises). Internals unchanged: one RLock, the folded snapshot fencing, and os.fork() restart all carry over. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/python/README.md | 88 ++--- packages/python/genai_prices/__init__.py | 4 - packages/python/genai_prices/update_prices.py | 315 +++++++----------- tests/test_update_prices.py | 309 +++++++---------- 4 files changed, 303 insertions(+), 413 deletions(-) diff --git a/packages/python/README.md b/packages/python/README.md index 3b60dfe4..ecb19b04 100644 --- a/packages/python/README.md +++ b/packages/python/README.md @@ -87,9 +87,9 @@ price = extracted_usage.calc_price() print(price.total_price) ``` -### Updating prices in the background +### `UpdatePrices` -`update_prices_in_background()` periodically updates the price data by downloading it from GitHub. +`UpdatePrices` periodically updates the price data by downloading it from GitHub. Please note: @@ -99,54 +99,68 @@ Please note: At the time of writing, the `data.json` file downloaded is around 26KB when compressed, so is generally very quick to download. By default the first fetch happens immediately in the background, then every hour after that. +Usage as a context manager: + ```py -from genai_prices import update_prices_in_background +from genai_prices import UpdatePrices, Usage, calc_price -handle = update_prices_in_background() -... -handle.close() # stop updating when you no longer need it +with UpdatePrices() as update_prices: + update_prices.wait() # optionally wait for prices to have updated + p = calc_price(Usage(input_tokens=123, output_tokens=456), 'gpt-5') + print(p) ``` -A single shared, process-wide updater backs this function, so it is safe to call from anywhere — including from -multiple libraries in the same process. The first call starts the updater; later calls reuse it and return -independent handles. The updater is stopped only when the **last** handle is closed, at which point prices revert -to the data bundled with the installed package. The handle is also a context manager (`with -update_prices_in_background() as handle: ...`). +Or by calling `start()` / `stop()` yourself: -**For libraries and integrations** (e.g. Logfire, Pydantic AI): call `update_prices_in_background()` with no -arguments, once, at startup. If several libraries do this, they share the one updater rather than spawning -duplicate threads, and each library's handle independently keeps it alive until closed. Leave configuration to -the application. +```py +from genai_prices import UpdatePrices, Usage, calc_price + +update_prices = UpdatePrices() +update_prices.start(wait=True) # optionally wait for the first update +p = calc_price(Usage(input_tokens=123, output_tokens=456), 'gpt-5') +print(p) +update_prices.stop() +``` -**For application authors** who need a custom URL or refresh interval: pass them, and call early in startup, +A single shared, process-wide updater backs every `UpdatePrices` instance, so it is safe to create and start them +from anywhere — including from several libraries in the same process. Starting an instance is a **claim** on that +one updater, not a private thread: the first `start()` launches it, later ones join it, and the background thread +stops (reverting prices to the data bundled with the installed package) only when the **last** started instance is +stopped. + +**For libraries and integrations** (e.g. Logfire, Pydantic AI): create one `UpdatePrices()` with no arguments, +`start()` it at startup, and `stop()` it on shutdown. If several libraries do this they share the one updater +rather than spawning duplicate threads, and each one's claim independently keeps it alive until stopped. Leave +configuration to the application. + +**For application authors** who need a custom URL or refresh interval: pass them, and start early in startup, before the libraries initialize: ```py -update_prices_in_background(url='https://my-mirror.example/prices.json', update_interval=1800) +UpdatePrices(url='https://my-mirror.example/prices.json', update_interval=1800).start() ``` -Configuration is **first-wins**: the first caller's `url`, `update_interval` and `request_timeout` apply for the -lifetime of the shared updater. A later caller passing different settings gets a handle on the already-running -updater and a warning logged on the `genai-prices` logger — its settings are ignored. Calling early ensures your -configuration is the one that takes effect. +Configuration is **first-wins**: the first instance to start fixes `url`, `update_interval` and `request_timeout` +for the updater's lifetime. Starting another instance with different settings logs a warning on the `genai-prices` +logger and joins the running updater; its settings are ignored. Starting early ensures your configuration is the +one that takes effect. -`UpdatePricesHandle.close()` is idempotent and never raises; errors from the background updater are logged -instead. Closing the last handle stops the updater and reverts prices to the bundled data immediately. If a -fetch is in flight, the daemon thread is given a short grace period to exit and is otherwise abandoned with a -warning log: it exits once the fetch completes, and its result is discarded — a stopped updater can never -install prices afterwards. Other updater API calls are never blocked, and `calc_price` is always unaffected. A -handle represents exactly one claim on the updater — don't copy one, as closing the copy releases the original's -claim too. +`stop()` is idempotent and never raises — a background fetch error is logged instead (use `wait()` if you want it +raised). Stopping the last claim reverts prices to the bundled data immediately. If a fetch is in flight, the +daemon thread is given a short grace period to exit and is otherwise abandoned with a warning log: it exits once +the fetch completes, and its result is discarded — a stopped updater can never install prices afterwards. Other +updater API calls are never blocked, and `calc_price` is always unaffected. -`update_prices_in_background()` does not wait for the download. Until the first fetch completes, `calc_price` +`start()` does not wait for the download (unless you pass `wait`). Until the first fetch completes, `calc_price` keeps using the data bundled with the installed package, so prices for models released after that snapshot may be missing for the first moments of the process. Once the fetch lands, every subsequent calculation uses the fresh data — prices computed before then are not recalculated. If you need fresh prices before calculating (e.g. in a -short-lived script), call `wait_prices_updated_sync()` / `wait_prices_updated_async()` after acquiring the -handle — these never raise and return `False` if the update failed (the error is logged on the `genai-prices` -logger), so your calculations simply fall back to the bundled data. +short-lived script), pass `wait` to `start()`, or call `wait_prices_updated_sync()` / +`wait_prices_updated_async()` — these never raise and return `False` if the update failed (the error is logged on +the `genai-prices` logger), so your calculations simply fall back to the bundled data. -You can wait for prices to be updated from anywhere with `wait_prices_updated_sync`: +You can wait for prices to be updated from anywhere — without access to the `UpdatePrices` instance — with +`wait_prices_updated_sync`: ```py from genai_prices import wait_prices_updated_sync @@ -157,14 +171,6 @@ wait_prices_updated_sync() Or its async variant, `wait_prices_updated_async`. -#### Deprecated: the `UpdatePrices` class - -The `UpdatePrices` class (`UpdatePrices().start()` / `.stop()` / `with UpdatePrices():`) is **deprecated** and -emits a `DeprecationWarning`. It now routes through the same shared updater described above and will be removed in -a future release — use `update_prices_in_background()` instead. Note the behaviour change: because there is now a -single shared updater, starting a second `UpdatePrices` no longer raises and no longer takes over an updater -another caller already started; configuration is first-wins. - ### CLI Usage Run the CLI with: diff --git a/packages/python/genai_prices/__init__.py b/packages/python/genai_prices/__init__.py index 2764f994..ac7f7812 100644 --- a/packages/python/genai_prices/__init__.py +++ b/packages/python/genai_prices/__init__.py @@ -8,8 +8,6 @@ from .types import Usage from .update_prices import ( UpdatePrices, - UpdatePricesHandle, - update_prices_in_background, wait_prices_updated_async, wait_prices_updated_sync, ) @@ -21,8 +19,6 @@ 'UpdatePrices', 'wait_prices_updated_sync', 'wait_prices_updated_async', - 'update_prices_in_background', - 'UpdatePricesHandle', '__version__', ) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index a0fc9a30..2177da63 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -4,7 +4,6 @@ import logging import os import threading -import warnings from dataclasses import dataclass, field from time import time @@ -15,8 +14,6 @@ __all__ = ( 'DEFAULT_UPDATE_URL', 'UpdatePrices', - 'UpdatePricesHandle', - 'update_prices_in_background', 'wait_prices_updated_sync', 'wait_prices_updated_async', ) @@ -31,9 +28,10 @@ def _default_request_timeout() -> httpx2.Timeout: # There is exactly one price snapshot per process (data_snapshot.set_custom_snapshot is a global), -# so "updating prices" is inherently a singleton activity. The whole updater is therefore a single -# process-wide, ref-counted instance guarded by `_lock`: every caller of update_prices_in_background() -# gets an independent handle on the shared updater, and the updater stops when the last handle closes. +# so "updating prices" is inherently a singleton activity. Every `UpdatePrices` is therefore not an +# updater of its own but a *claim* on one shared, process-wide updater guarded by `_lock`: starting +# any instance acquires a claim (ref-count++), and the background thread stops only when the last +# claim is released. # # This module-level state would not survive os.fork() on its own (threads die with the parent, and # locks can be inherited in a held state, e.g. under gunicorn with preload_app=True). Fork hooks @@ -80,7 +78,7 @@ def _fork_after_in_child() -> None: return try: # The parent's updater thread does not survive the fork; restart it on the same instance, - # preserving identity so existing handles remain valid. + # preserving identity so existing claims remain valid. updater._revive_after_fork() # pyright: ignore[reportPrivateUsage] except Exception: _updater = None @@ -111,72 +109,6 @@ def _join_stopped_updater_thread(thread: threading.Thread | None) -> None: ) -def update_prices_in_background( - *, - url: str = DEFAULT_UPDATE_URL, - update_interval: float = DEFAULT_UPDATE_INTERVAL, - request_timeout: httpx2.Timeout | None = None, -) -> UpdatePricesHandle: - """Update prices in the background using a shared, process-wide daemon thread. - - The first call starts the shared updater with the given configuration; later calls reuse it - and return independent handles. The updater is stopped when the last handle is closed, at - which point prices revert to the data bundled with the installed package. - - Configuration is first-wins: the first caller's `url`, `update_interval` and `request_timeout` - apply for the lifetime of the shared updater. A later caller passing different configuration - gets a warning and a handle on the already-running updater - its configuration is ignored. App - authors who need a custom URL should call this early in startup, before any library does. - - This function does not wait for the download: until the first fetch completes, price - calculations keep using the bundled data, and prices computed before then are not recalculated - once fresh data lands. Use `wait_prices_updated_sync` or `wait_prices_updated_async` if you - need fresh prices before calculating. - - Returns: - A handle that stops the shared background updater when all handles have been closed. - """ - global _updater, _ref_count - - if request_timeout is None: - request_timeout = _default_request_timeout() - - with _lock: - if _updater is not None: - if ( - _updater.url != url - or _updater.update_interval != update_interval - or _updater.request_timeout != request_timeout - ): - logger.warning( - 'A genai-prices background updater is already running (url=%r, update_interval=%r, ' - 'request_timeout=%r); ignoring the different configuration passed to this call ' - '(url=%r, update_interval=%r, request_timeout=%r) and returning a handle on the ' - 'existing updater. Configure the updater before any other caller starts it if you ' - 'need custom settings.', - _updater.url, - _updater.update_interval, - _updater.request_timeout, - url, - update_interval, - request_timeout, - ) - _ref_count += 1 - return UpdatePricesHandle(_updater) - - updater = _BackgroundUpdater(url=url, update_interval=update_interval, request_timeout=request_timeout) - _updater = updater - _ref_count = 1 - try: - updater._start_thread() # pyright: ignore[reportPrivateUsage] - except Exception: - _updater = None - _ref_count = 0 - raise - - return UpdatePricesHandle(updater) - - def wait_prices_updated_sync(timeout: float | None = None) -> bool: """Synchronously wait for prices to be updated by the shared background updater. @@ -216,78 +148,159 @@ async def wait_prices_updated_async(timeout: float | None = None) -> bool: @dataclass -class UpdatePricesHandle: - """A claim on the shared background updater started by `update_prices_in_background`. +class UpdatePrices: + """Periodically update price data by downloading it in a background daemon thread. - A handle only releases the updater it was created for: handles constructed directly, or - outliving the updater they belong to, are inert and closing them does nothing. + A single shared, process-wide updater backs every `UpdatePrices` instance: starting an instance + is a *claim* on that one updater, not a private thread. It is therefore safe to create and start + `UpdatePrices` from anywhere - including from several libraries in the same process. The first + `start()` launches the updater; later ones join it and bump a reference count; the background + thread stops (and prices revert to the bundled data) only when the **last** started instance is + stopped. - Do not copy a handle (`copy.copy`, `dataclasses.replace`, pickling): each handle represents - exactly one claim, so closing a copy releases the original's claim and can stop the shared - updater while other consumers still hold open handles. + Configuration is first-wins: the first instance to start fixes `url`, `update_interval` and + `request_timeout` for the updater's lifetime. Starting another instance with different settings + logs a warning and joins the running updater; its settings are ignored. Application authors who + need a custom URL should start their instance early, before any library does. - Can be used as a context manager; exiting the block closes the handle. + Can be used as a context manager (`with UpdatePrices(): ...`) or by calling `start()`/`stop()`. """ - _update_prices: _BackgroundUpdater | None = None - _closed: bool = field(default=False, init=False) + update_interval: float = DEFAULT_UPDATE_INTERVAL + """How often to update prices in seconds.""" + url: str = DEFAULT_UPDATE_URL + """The URL to fetch prices from.""" + request_timeout: httpx2.Timeout = field(default_factory=_default_request_timeout) + """The timeout for HTTP requests.""" + _claimed: bool = field(default=False, init=False) + _updater: _BackgroundUpdater | None = field(default=None, init=False) - def close(self): - """Release this handle's claim on the shared updater. + def start(self, *, wait: bool | float = False): + """Acquire this instance's claim on the shared background updater. - Stops the updater if this was the last open handle. Idempotent, and never raises: - errors from the background updater are logged instead. + The first claim starts the updater; later claims join it. Starting the same instance twice + raises `RuntimeError`. This does not wait for the download unless `wait` is passed: until + the first fetch completes, price calculations keep using the bundled data. - Returns promptly: prices revert to the bundled data immediately, and if a fetch is in - flight the daemon thread is given a short grace period to exit before being abandoned - with a warning log - a stopped updater can never install its result afterwards. + Args: + wait: Whether to wait for prices to be updated before returning; if an int/float is + passed wait that many seconds, if `True` wait for 30 seconds. """ - exc = self._release() - if exc is not None: - logger.error('Error from genai-prices background updater while closing', exc_info=exc) + global _updater, _ref_count - def _release(self) -> Exception | None: - """Release this handle's claim and tear down the updater if it was the last one. + with _lock: + if self._claimed: + raise RuntimeError('UpdatePrices background task already started') + + if _updater is None: + updater = _BackgroundUpdater( + url=self.url, update_interval=self.update_interval, request_timeout=self.request_timeout + ) + _updater = updater + _ref_count = 1 + try: + updater._start_thread() # pyright: ignore[reportPrivateUsage] + except Exception: + _updater = None + _ref_count = 0 + raise + else: + if ( + _updater.url != self.url + or _updater.update_interval != self.update_interval + or _updater.request_timeout != self.request_timeout + ): + logger.warning( + 'A genai-prices background updater is already running (url=%r, update_interval=%r, ' + 'request_timeout=%r); ignoring the different configuration of this UpdatePrices ' + '(url=%r, update_interval=%r, request_timeout=%r) and joining the existing updater. ' + 'Start the updater before any other caller if you need custom settings.', + _updater.url, + _updater.update_interval, + _updater.request_timeout, + self.url, + self.update_interval, + self.request_timeout, + ) + _ref_count += 1 + + self._claimed = True + self._updater = _updater + + if wait: + self.wait(timeout=30 if wait is True else wait) - Returns the stored background exception (if any) for the caller to surface: the public - `close()` logs it, while the deprecated `UpdatePrices.stop()` re-raises it. Idempotent. + def stop(self): + """Release this instance's claim on the shared background updater. + + Stops the updater (reverting prices to the bundled data) only if this was the last open + claim; a claim held by another caller keeps it running. A no-op if this instance was never + started or has already been stopped. + + Never raises: a stored background fetch error is logged instead - use `wait()` if you want + the error raised. Returns promptly; if a fetch is in flight the daemon thread is given a + short grace period to exit before being abandoned with a warning log, and its result is + discarded - a stopped updater can never install prices afterwards. """ global _updater, _ref_count with _lock: - if self._closed: - return None + if not self._claimed: + return + self._claimed = False - self._closed = True - if self._update_prices is None or _updater is not self._update_prices: - return None + updater = self._updater + self._updater = None + if updater is None or _updater is not updater: + return _ref_count -= 1 if _ref_count > 0: - return None + return - update_prices = self._update_prices _updater = None _ref_count = 0 - thread = update_prices._stop_and_detach() # pyright: ignore[reportPrivateUsage] + thread = updater._stop_and_detach() # pyright: ignore[reportPrivateUsage] # Join outside the lock so a thread blocked on an in-flight fetch never stalls other # updater API calls; re-publication is already fenced off by _stop_and_detach(). _join_stopped_updater_thread(thread) - exc = update_prices._background_exc # pyright: ignore[reportPrivateUsage] - update_prices._background_exc = None # pyright: ignore[reportPrivateUsage] - return exc + exc = updater._background_exc # pyright: ignore[reportPrivateUsage] + if exc: + updater._background_exc = None # pyright: ignore[reportPrivateUsage] + logger.error('Error from genai-prices background updater while stopping', exc_info=exc) + + def wait(self, timeout: float | None = None) -> bool: + """Wait for prices to be updated by the shared background updater. + + Raises the stored exception if the update attempt failed. Returns False if this instance is + not currently started. + + Args: + timeout: The maximum time to wait for the prices to be updated in seconds. + """ + updater = self._updater + if updater is None: + return False + return updater._wait_raising(timeout) # pyright: ignore[reportPrivateUsage] + + def fetch(self) -> data_snapshot.DataSnapshot | None: + """Fetch the latest provider data from the configured URL (does not start a background task).""" + return _BackgroundUpdater( + url=self.url, update_interval=self.update_interval, request_timeout=self.request_timeout + ).fetch() - def __enter__(self) -> UpdatePricesHandle: + def __enter__(self): + self.start() return self def __exit__(self, *_args: object): - self.close() + self.stop() @dataclass class _BackgroundUpdater: - """The single process-wide updater. Internal: consumers interact via `UpdatePricesHandle`.""" + """The single process-wide updater. Internal: consumers interact via `UpdatePrices`.""" url: str update_interval: float @@ -312,12 +325,12 @@ def _start_thread(self) -> None: def _wait_updated(self, timeout: float | None) -> bool: # Never raises and does not consume the stored background exception: failures are already - # logged by the background task, and the exception stays stored for handle close to surface. + # logged by the background task, and the exception stays stored for stop() to surface. return self._prices_updated.wait(timeout=timeout) and self._update_succeeded def _wait_raising(self, timeout: float | None) -> bool: # Like _wait_updated, but raises (and consumes) the stored background exception - the - # historical UpdatePrices.wait() behaviour. + # behaviour of UpdatePrices.wait(). prices_updated = self._prices_updated.wait(timeout=timeout) exc = self._background_exc if exc: @@ -326,7 +339,7 @@ def _wait_raising(self, timeout: float | None) -> bool: return prices_updated and self._update_succeeded def _revive_after_fork(self) -> None: - # Threads do not survive fork, and the inherited events/locks may have been captured in an + # Threads do not survive fork, and the inherited events may have been captured in an # arbitrary state (e.g. mid-install); recreate them and restart the background thread on # this same instance. The snapshot is fenced by the module `_lock`, which is itself # replaced with a fresh one in `_fork_after_in_child`. @@ -397,77 +410,3 @@ def fetch(self) -> data_snapshot.DataSnapshot | None: r = httpx2.get(self.url, timeout=self.request_timeout) r.raise_for_status() return data_snapshot.DataSnapshot(data.providers_schema.validate_json(r.content), from_auto_update=True) - - -@dataclass -class UpdatePrices: - """Deprecated. Use `update_prices_in_background()` instead. - - Retained as a thin shim over the shared, process-wide updater so existing code keeps working. - It will be removed in a future release. - - Behavioural notes vs. the historical class: there is now a single shared updater, so a second - `start()` no longer raises and starting an instance no longer takes over (and stops) an updater - that another caller already started - configuration is first-wins. See `update_prices_in_background`. - """ - - update_interval: float = DEFAULT_UPDATE_INTERVAL - """How often to update prices in seconds.""" - url: str = DEFAULT_UPDATE_URL - """The URL to fetch prices from.""" - request_timeout: httpx2.Timeout = field(default_factory=_default_request_timeout) - """The timeout for HTTP requests.""" - _handle: UpdatePricesHandle | None = field(default=None, init=False) - - def start(self, *, wait: bool | float = False): - """Start the shared background updater (deprecated; use `update_prices_in_background()`).""" - warnings.warn( - 'UpdatePrices is deprecated; use update_prices_in_background() instead.', - DeprecationWarning, - stacklevel=2, - ) - if self._handle is not None and not self._handle._closed: # pyright: ignore[reportPrivateUsage] - raise RuntimeError('UpdatePrices background task already started') - - self._handle = update_prices_in_background( - url=self.url, update_interval=self.update_interval, request_timeout=self.request_timeout - ) - if wait: - self.wait(timeout=30 if wait is True else wait) - - def wait(self, timeout: float | None = None) -> bool: - """Wait for the prices to be updated by the shared background updater. - - Raises the stored exception if the update attempt failed. - """ - updater = self._handle._update_prices if self._handle is not None else None # pyright: ignore[reportPrivateUsage] - if updater is None: - return False - return updater._wait_raising(timeout) # pyright: ignore[reportPrivateUsage] - - def fetch(self) -> data_snapshot.DataSnapshot | None: - """Fetch the latest provider data from the configured URL (does not start a background task).""" - return _BackgroundUpdater( - url=self.url, update_interval=self.update_interval, request_timeout=self.request_timeout - ).fetch() - - def stop(self): - """Release this instance's claim on the shared updater (deprecated). - - Stops the shared updater only if this was the last open claim; under the shared model a - claim held by another caller keeps it running. Re-raises a stored background fetch error, - preserving the historical `UpdatePrices.stop()` behaviour. - """ - if self._handle is None: - return - exc = self._handle._release() # pyright: ignore[reportPrivateUsage] - self._handle = None - if exc is not None: - raise exc - - def __enter__(self): - self.start() - return self - - def __exit__(self, *_args: object): - self.stop() diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 27197458..47dd59de 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -9,12 +9,10 @@ from genai_prices import ( UpdatePrices, - UpdatePricesHandle, Usage, calc_price, data_snapshot, update_prices as update_prices_module, - update_prices_in_background, wait_prices_updated_async, wait_prices_updated_sync, ) @@ -57,7 +55,6 @@ def test_update_prices_fetch_parses_provider_array(monkeypatch: pytest.MonkeyPat _mock_update_prices_get(monkeypatch, content) - # fetch() does not emit a deprecation warning (only start() does). snapshot = UpdatePrices(url='https://example.test/prices.json').fetch() assert snapshot is not None @@ -67,11 +64,11 @@ def test_update_prices_fetch_parses_provider_array(monkeypatch: pytest.MonkeyPat assert model.id == 'gpt-4o-mini' -def test_background_update_and_calc_price(monkeypatch: pytest.MonkeyPatch): +def test_update_prices_wait_on_start(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) assert data_snapshot._custom_snapshot is None - with update_prices_in_background(): - assert wait_prices_updated_sync(timeout=5) + with UpdatePrices() as update_prices: + update_prices.wait() assert data_snapshot._custom_snapshot is not None price = calc_price(Usage(input_tokens=1000, output_tokens=100), model_ref='gpt-4o', provider_id='openai') assert price.input_price == snapshot(Decimal('0.0025')) @@ -82,38 +79,125 @@ def test_background_update_and_calc_price(monkeypatch: pytest.MonkeyPatch): assert data_snapshot._custom_snapshot is None +def test_wait_prices_updated_sync(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + assert data_snapshot._custom_snapshot is None + with UpdatePrices(): + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + assert data_snapshot._custom_snapshot is None + + async def test_wait_prices_updated_async(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) assert data_snapshot._custom_snapshot is None - with update_prices_in_background(): + with UpdatePrices(): assert await wait_prices_updated_async(timeout=5) assert data_snapshot._custom_snapshot is not None assert data_snapshot._custom_snapshot is None +def test_ref_counted_across_instances(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + up1 = UpdatePrices() + up2 = UpdatePrices() + up1.start() + up2.start() + try: + assert wait_prices_updated_sync(timeout=5) + assert data_snapshot._custom_snapshot is not None + + # Releasing one claim keeps the updater alive for the other. + up1.stop() + assert wait_prices_updated_sync(timeout=0) + assert data_snapshot._custom_snapshot is not None + + up1.stop() # idempotent no-op + assert data_snapshot._custom_snapshot is not None + + up2.stop() + assert data_snapshot._custom_snapshot is None + assert not wait_prices_updated_sync(timeout=0) + finally: + up1.stop() + up2.stop() + data_snapshot.set_custom_snapshot(None) + + def test_first_wins_config(monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture): _mock_update_prices_get(monkeypatch) - handle_1 = update_prices_in_background(url='https://example.test/prices.json') + up1 = UpdatePrices(url='https://example.test/prices.json') + up1.start() try: with caplog.at_level('WARNING', logger='genai-prices'): - handle_2 = update_prices_in_background(url=DEFAULT_UPDATE_URL, update_interval=1) + up2 = UpdatePrices(url=DEFAULT_UPDATE_URL, update_interval=1) + up2.start() try: - # The first caller's configuration wins; the second joins the running updater. - assert handle_1 is not handle_2 + # The first instance's configuration wins; the second joins the running updater. assert update_prices_module._updater is not None assert update_prices_module._updater.url == 'https://example.test/prices.json' assert update_prices_module._updater.update_interval == 3600 assert any('already running' in record.message for record in caplog.records) assert wait_prices_updated_sync(timeout=5) finally: - handle_2.close() + up2.stop() + finally: + up1.stop() + data_snapshot.set_custom_snapshot(None) + assert data_snapshot._custom_snapshot is None + + +def test_double_start_same_instance_raises(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + update_prices = UpdatePrices() + update_prices.start() + try: + with pytest.raises(RuntimeError, match='already started'): + update_prices.start() finally: - handle_1.close() + update_prices.stop() data_snapshot.set_custom_snapshot(None) + + +def test_stop_on_unstarted_instance_is_noop(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + with UpdatePrices() as update_prices: + assert update_prices.wait(timeout=5) + # stop() on a never-started instance does not touch the live updater. + UpdatePrices().stop() + assert update_prices_module._updater is not None + assert data_snapshot._custom_snapshot is not None assert data_snapshot._custom_snapshot is None -def test_close_does_not_block_on_in_flight_fetch_and_discards_result( +def test_restopping_does_not_affect_a_new_updater(monkeypatch: pytest.MonkeyPatch): + _mock_update_prices_get(monkeypatch) + up1 = UpdatePrices() + up1.start() + assert wait_prices_updated_sync(timeout=5) + stale_updater = update_prices_module._updater + up1.stop() + assert data_snapshot._custom_snapshot is None + + up2 = UpdatePrices() + up2.start() + try: + assert wait_prices_updated_sync(timeout=5) + new_updater = update_prices_module._updater + assert new_updater is not None and new_updater is not stale_updater + + # up1 was already released; stopping it again must not release up2's claim. + up1.stop() + assert update_prices_module._updater is new_updater + assert update_prices_module._ref_count == 1 + assert data_snapshot._custom_snapshot is not None + finally: + up2.stop() + data_snapshot.set_custom_snapshot(None) + assert data_snapshot._custom_snapshot is None + + +def test_stop_does_not_block_on_in_flight_fetch_and_discards_result( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ): fetch_started = threading.Event() @@ -135,14 +219,15 @@ def fake_get(url: str, timeout: httpx2.Timeout) -> Response: monkeypatch.setattr(httpx2, 'get', fake_get) monkeypatch.setattr(update_prices_module, '_STOPPED_THREAD_JOIN_TIMEOUT', 0.05) - handle = update_prices_in_background() + update_prices = UpdatePrices() + update_prices.start() try: assert fetch_started.wait(timeout=5) (thread,) = [t for t in threading.enumerate() if t.name == 'genai_prices:update'] start = monotonic() with caplog.at_level('WARNING', logger='genai-prices'): - handle.close() + update_prices.stop() assert monotonic() - start < 2 # did not wait for the in-flight fetch assert any('abandoning the daemon thread' in record.message for record in caplog.records) assert data_snapshot._custom_snapshot is None @@ -154,99 +239,40 @@ def fake_get(url: str, timeout: httpx2.Timeout) -> Response: assert data_snapshot._custom_snapshot is None finally: allow_fetch_return.set() - handle.close() - data_snapshot.set_custom_snapshot(None) - - -def test_update_prices_in_background_ref_count(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - handle_1 = update_prices_in_background() - handle_2 = update_prices_in_background() - try: - assert handle_1 is not handle_2 - assert wait_prices_updated_sync(timeout=5) - assert data_snapshot._custom_snapshot is not None - - handle_1.close() - assert wait_prices_updated_sync(timeout=0) - assert data_snapshot._custom_snapshot is not None - - handle_1.close() # idempotent - assert wait_prices_updated_sync(timeout=0) - assert data_snapshot._custom_snapshot is not None - - handle_2.close() - assert data_snapshot._custom_snapshot is None - assert not wait_prices_updated_sync(timeout=0) - finally: - handle_1.close() - handle_2.close() - data_snapshot.set_custom_snapshot(None) - - -def test_update_prices_handle_directly_constructed_is_inert(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - handle = update_prices_in_background() - try: - assert wait_prices_updated_sync(timeout=5) - - UpdatePricesHandle().close() - assert wait_prices_updated_sync(timeout=0) - assert data_snapshot._custom_snapshot is not None - finally: - handle.close() + update_prices.stop() data_snapshot.set_custom_snapshot(None) - assert data_snapshot._custom_snapshot is None - -def test_stale_handle_close_does_not_affect_new_shared_updater(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - handle_1 = update_prices_in_background() - assert wait_prices_updated_sync(timeout=5) - stale_updater = update_prices_module._updater - assert stale_updater is not None - # Fully release the first updater, then start a fresh one. - handle_1.close() +@pytest.mark.default_cassette('fail.yaml') +@pytest.mark.vcr() +def test_wait_raises_on_failed_fetch(): assert data_snapshot._custom_snapshot is None - - handle_2 = update_prices_in_background() - try: - assert wait_prices_updated_sync(timeout=5) - new_updater = update_prices_module._updater - assert new_updater is not None and new_updater is not stale_updater - - # A handle bound to the now-defunct first updater must not release handle_2's claim. - UpdatePricesHandle(stale_updater).close() - assert update_prices_module._updater is new_updater - assert update_prices_module._ref_count == 1 - assert wait_prices_updated_sync(timeout=0) - assert data_snapshot._custom_snapshot is not None - finally: - handle_2.close() - data_snapshot.set_custom_snapshot(None) + with UpdatePrices(url='https://demo-endpoints.pydantic.workers.dev/bin?status=404') as update_prices: + with pytest.raises(httpx2.HTTPStatusError): + update_prices.wait(timeout=5) assert data_snapshot._custom_snapshot is None -def test_update_prices_handle_close_does_not_raise_after_failed_fetch( +def test_stop_logs_and_does_not_raise_after_failed_fetch( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ): def fake_get(*_args: object, **_kwargs: object) -> object: raise httpx2.ConnectError('network down') monkeypatch.setattr(httpx2, 'get', fake_get) - handle = update_prices_in_background() + update_prices = UpdatePrices() + update_prices.start() try: - # Wait on the internal event rather than wait_prices_updated_sync, which would - # consume the stored exception that close() is expected to log instead of raise. + # Wait on the internal event rather than wait(), which would consume and raise the stored + # exception that stop() is expected to log instead. updater = update_prices_module._updater assert updater is not None assert updater._prices_updated.wait(timeout=5) finally: with caplog.at_level('ERROR', logger='genai-prices'): - handle.close() + update_prices.stop() # never raises data_snapshot.set_custom_snapshot(None) - assert any('while closing' in record.message for record in caplog.records) + assert any('while stopping' in record.message for record in caplog.records) def test_wait_prices_updated_sync_returns_false_on_failed_fetch( @@ -256,7 +282,8 @@ def fake_get(*_args: object, **_kwargs: object) -> object: raise httpx2.ConnectError('network down') monkeypatch.setattr(httpx2, 'get', fake_get) - handle = update_prices_in_background() + update_prices = UpdatePrices() + update_prices.start() try: # Never raises and returns False on failure, for every waiter — not just the first. assert wait_prices_updated_sync(timeout=5) is False @@ -264,16 +291,17 @@ def fake_get(*_args: object, **_kwargs: object) -> object: assert data_snapshot._custom_snapshot is None finally: with caplog.at_level('ERROR', logger='genai-prices'): - handle.close() + update_prices.stop() data_snapshot.set_custom_snapshot(None) - # The stored exception is not consumed by the waiters above, so close() still logs it. - assert any('while closing' in record.message for record in caplog.records) + # The stored exception is not consumed by the waiters above, so stop() still logs it. + assert any('while stopping' in record.message for record in caplog.records) @pytest.mark.skipif(not hasattr(os, 'fork'), reason='requires os.fork') def test_forked_child_restarts_shared_updater(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) - handle = update_prices_in_background() + update_prices = UpdatePrices() + update_prices.start() try: assert wait_prices_updated_sync(timeout=5) @@ -294,11 +322,11 @@ def test_forked_child_restarts_shared_updater(monkeypatch: pytest.MonkeyPatch): _, status = os.waitpid(pid, 0) assert os.waitstatus_to_exitcode(status) == 0 finally: - handle.close() + update_prices.stop() data_snapshot.set_custom_snapshot(None) -def test_update_prices_in_background_concurrent_acquire_close(monkeypatch: pytest.MonkeyPatch): +def test_concurrent_start_stop(monkeypatch: pytest.MonkeyPatch): _mock_update_prices_get(monkeypatch) barrier = threading.Barrier(16) errors: list[BaseException] = [] @@ -307,8 +335,9 @@ def worker() -> None: try: barrier.wait(timeout=5) for _ in range(10): - handle = update_prices_in_background() - handle.close() + update_prices = UpdatePrices() + update_prices.start() + update_prices.stop() except BaseException as exc: # pragma: no cover errors.append(exc) @@ -328,91 +357,11 @@ def worker() -> None: assert data_snapshot._custom_snapshot is None # The shared updater can still be acquired cleanly after the churn. - handle = update_prices_in_background() - try: - assert wait_prices_updated_sync(timeout=5) - assert data_snapshot._custom_snapshot is not None - finally: - handle.close() - data_snapshot.set_custom_snapshot(None) - - -# --- Deprecated UpdatePrices shim ------------------------------------------------------------ - - -def test_deprecated_update_prices_warns_and_routes_through_shared_updater(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - assert data_snapshot._custom_snapshot is None - with pytest.warns(DeprecationWarning, match='update_prices_in_background'): - with UpdatePrices() as update_prices: - assert update_prices.wait(timeout=5) - assert data_snapshot._custom_snapshot is not None - assert data_snapshot._custom_snapshot is None - - -def test_deprecated_update_prices_multiple_instances_no_longer_raise(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - # Two distinct instances now share the one updater (first-wins) instead of raising. - with pytest.warns(DeprecationWarning): - with UpdatePrices(): - with UpdatePrices(): - assert wait_prices_updated_sync(timeout=5) - assert data_snapshot._custom_snapshot is not None - assert data_snapshot._custom_snapshot is None - - -def test_deprecated_update_prices_same_instance_double_start_raises(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) update_prices = UpdatePrices() - with pytest.warns(DeprecationWarning): - update_prices.start() - try: - with pytest.warns(DeprecationWarning): - with pytest.raises(RuntimeError, match='already started'): - update_prices.start() - finally: - update_prices.stop() - data_snapshot.set_custom_snapshot(None) - - -@pytest.mark.default_cassette('fail.yaml') -@pytest.mark.vcr() -def test_deprecated_update_prices_wait_raises_on_failed_fetch(): - assert data_snapshot._custom_snapshot is None - update_prices = UpdatePrices(url='https://demo-endpoints.pydantic.workers.dev/bin?status=404') - with pytest.warns(DeprecationWarning): - update_prices.start() - try: - with pytest.raises(httpx2.HTTPStatusError): - update_prices.wait(timeout=5) - finally: - update_prices.stop() - assert data_snapshot._custom_snapshot is None - - -@pytest.mark.default_cassette('fail.yaml') -@pytest.mark.vcr() -def test_deprecated_update_prices_stop_raises_on_failed_fetch(): - # The deprecated stop() preserves the historical behaviour of re-raising a stored fetch error. - assert data_snapshot._custom_snapshot is None - update_prices = UpdatePrices(url='https://demo-endpoints.pydantic.workers.dev/bin?status=404') - with pytest.warns(DeprecationWarning): - update_prices.start() - with pytest.raises(httpx2.HTTPStatusError): - update_prices.stop() - assert data_snapshot._custom_snapshot is None - - -def test_deprecated_stop_on_unstarted_instance_is_noop(monkeypatch: pytest.MonkeyPatch): - _mock_update_prices_get(monkeypatch) - handle = update_prices_in_background() + update_prices.start() try: assert wait_prices_updated_sync(timeout=5) - # stop() on a never-started instance does not warn and does not touch the live updater. - UpdatePrices().stop() - assert update_prices_module._updater is not None assert data_snapshot._custom_snapshot is not None finally: - handle.close() + update_prices.stop() data_snapshot.set_custom_snapshot(None) - assert data_snapshot._custom_snapshot is None From c5762d1dff28dad2fe50fd7d0f6b942b960a4a7e Mon Sep 17 00:00:00 2001 From: Aditya Vardhan Date: Mon, 29 Jun 2026 09:58:08 +0530 Subject: [PATCH 17/17] Fix: discarded fetch must not report success to waiters (Codex review) Race found by Codex review: when stop() wins the fence and an in-flight fetch is discarded by _install_snapshot, the background loop still set _update_succeeded=True and signalled the event, so a waiter parked before the stop could get True while the snapshot had reverted to bundled data. _install_snapshot now returns whether it installed; the loop reflects that in _update_succeeded. Adds a deterministic regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/python/genai_prices/update_prices.py | 19 +++++--- tests/test_update_prices.py | 43 +++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/packages/python/genai_prices/update_prices.py b/packages/python/genai_prices/update_prices.py index 2177da63..87fca8ca 100644 --- a/packages/python/genai_prices/update_prices.py +++ b/packages/python/genai_prices/update_prices.py @@ -367,10 +367,12 @@ def _background_task(self) -> None: try: while True: try: - self._update_prices() + installed = self._update_prices() self._background_exc = None - # Set before signaling the event so waiters observing the event see the flag. - self._update_succeeded = True + # Reflect whether the snapshot was actually installed: a fetch discarded by the + # stop fence (see _install_snapshot) must not report success to waiters. Set + # before signaling the event so waiters observing the event see the flag. + self._update_succeeded = installed self._prices_updated.set() except Exception as e: self._background_exc = e @@ -382,7 +384,7 @@ def _background_task(self) -> None: finally: logger.info('genai-prices background task stopped') - def _update_prices(self): + def _update_prices(self) -> bool: start = time() snapshot = self.fetch() interval = time() - start @@ -391,17 +393,20 @@ def _update_prices(self): else: logger.info('Successfully fetched null snapshot in %.2f seconds', interval) - self._install_snapshot(snapshot) + return self._install_snapshot(snapshot) - def _install_snapshot(self, snapshot: data_snapshot.DataSnapshot | None) -> None: + def _install_snapshot(self, snapshot: data_snapshot.DataSnapshot | None) -> bool: # Fencing check: never publish after stop. The stop path sets _stop_event and then clears # the snapshot under `_lock` (see _stop_and_detach), so taking the same lock here and # re-checking the event makes publish-after-stop impossible rather than merely unlikely. + # Returns whether the snapshot was installed, so a discarded fetch is not reported as a + # successful update to waiters. with _lock: if self._stop_event.is_set(): logger.info('genai-prices updater was stopped during the fetch; discarding the fetched snapshot') - return + return False data_snapshot.set_custom_snapshot(snapshot) + return True def fetch(self) -> data_snapshot.DataSnapshot | None: """Fetches the latest provider data from the configured URL.""" diff --git a/tests/test_update_prices.py b/tests/test_update_prices.py index 47dd59de..265ecf30 100644 --- a/tests/test_update_prices.py +++ b/tests/test_update_prices.py @@ -243,6 +243,49 @@ def fake_get(url: str, timeout: httpx2.Timeout) -> Response: data_snapshot.set_custom_snapshot(None) +def test_waiter_gets_false_when_stop_discards_in_flight_fetch(monkeypatch: pytest.MonkeyPatch): + # A fetch discarded by the stop fence must not be reported to waiters as a successful update. + fetch_started = threading.Event() + allow_fetch_return = threading.Event() + + class Response: + content = PROVIDER_ARRAY_PAYLOAD + + def raise_for_status(self) -> None: + pass + + def fake_get(url: str, timeout: httpx2.Timeout) -> Response: + assert url == DEFAULT_UPDATE_URL + assert timeout is not None + fetch_started.set() + assert allow_fetch_return.wait(timeout=5) + return Response() + + monkeypatch.setattr(httpx2, 'get', fake_get) + monkeypatch.setattr(update_prices_module, '_STOPPED_THREAD_JOIN_TIMEOUT', 0.05) + + update_prices = UpdatePrices() + update_prices.start() + try: + assert fetch_started.wait(timeout=5) + # A waiter that captured the updater before stop() observes this instance directly. + updater = update_prices_module._updater + assert updater is not None + + update_prices.stop() # sets the stop fence and reverts before the fetch returns + allow_fetch_return.set() # the in-flight fetch now completes but must be discarded + + # The background loop signals the event but must report the discarded fetch as not-updated. + assert updater._prices_updated.wait(timeout=5) + assert updater._update_succeeded is False + assert updater._wait_updated(0) is False + assert data_snapshot._custom_snapshot is None + finally: + allow_fetch_return.set() + update_prices.stop() + data_snapshot.set_custom_snapshot(None) + + @pytest.mark.default_cassette('fail.yaml') @pytest.mark.vcr() def test_wait_raises_on_failed_fetch():