diff --git a/news/+envvar-durations.feature.md b/news/+envvar-durations.feature.md new file mode 100644 index 00000000000..74b29d738f0 --- /dev/null +++ b/news/+envvar-durations.feature.md @@ -0,0 +1 @@ +The duration settings read by the app and the state managers take a unit suffix: `SQLALCHEMY_POOL_TIMEOUT`, `REFLEX_SOCKET_INTERVAL` and `REFLEX_SOCKET_TIMEOUT` accept values such as `2m`, and `REFLEX_AUTO_RELOAD_COOLDOWN`, `REFLEX_OPLOCK_HOLD_TIME` and `REFLEX_STATE_MANAGER_DISK_DEBOUNCE` replace the `_MS`/`_SECONDS` names, which still work with a deprecation warning until 1.0. A bare number is read as seconds. diff --git a/packages/reflex-base/news/+deprecation-location-synthetic-frames.bugfix.md b/packages/reflex-base/news/+deprecation-location-synthetic-frames.bugfix.md new file mode 100644 index 00000000000..016ca5f9ca7 --- /dev/null +++ b/packages/reflex-base/news/+deprecation-location-synthetic-frames.bugfix.md @@ -0,0 +1 @@ +Deprecation warnings no longer point at a pseudo-location such as `` or `` when the deprecated call runs inside generated or frozen code; the location now names the first real user file. diff --git a/packages/reflex-base/news/+envvar-duration-deprecations.feature.md b/packages/reflex-base/news/+envvar-duration-deprecations.feature.md new file mode 100644 index 00000000000..21343b77093 --- /dev/null +++ b/packages/reflex-base/news/+envvar-duration-deprecations.feature.md @@ -0,0 +1 @@ +`REFLEX_AUTO_RELOAD_COOLDOWN`, `REFLEX_OPLOCK_HOLD_TIME` and `REFLEX_STATE_MANAGER_DISK_DEBOUNCE` replace `REFLEX_AUTO_RELOAD_COOLDOWN_TIME_MS`, `REFLEX_OPLOCK_HOLD_TIME_MS` and `REFLEX_STATE_MANAGER_DISK_DEBOUNCE_SECONDS`, and take a duration such as `250ms` or `5m`. The old names still work and keep counting the unit in their name, with a deprecation warning; they are removed in 1.0. diff --git a/packages/reflex-base/news/+envvar-timedelta-usages.feature.md b/packages/reflex-base/news/+envvar-timedelta-usages.feature.md new file mode 100644 index 00000000000..b8107bf36a7 --- /dev/null +++ b/packages/reflex-base/news/+envvar-timedelta-usages.feature.md @@ -0,0 +1 @@ +`SQLALCHEMY_POOL_TIMEOUT`, `REFLEX_BACKEND_COLD_START_TIMEOUT`, `REFLEX_SOCKET_INTERVAL` and `REFLEX_SOCKET_TIMEOUT` are `timedelta` settings, so they accept a unit suffix such as `REFLEX_SOCKET_TIMEOUT=2m`. A bare number is still read as seconds, so existing values keep their meaning. diff --git a/packages/reflex-base/src/reflex_base/environment.py b/packages/reflex-base/src/reflex_base/environment.py index c937d0e9b34..7ddbd7b4073 100644 --- a/packages/reflex-base/src/reflex_base/environment.py +++ b/packages/reflex-base/src/reflex_base/environment.py @@ -697,7 +697,7 @@ class EnvironmentVariables: SQLALCHEMY_POOL_RECYCLE: EnvVar[int] = env_var(-1) # The timeout for acquiring a connection from the pool. - SQLALCHEMY_POOL_TIMEOUT: EnvVar[int] = env_var(30) + SQLALCHEMY_POOL_TIMEOUT: EnvVar[timedelta] = env_var(timedelta(seconds=30)) # Whether to ignore the redis config error. Some redis servers only allow out-of-band configuration. REFLEX_IGNORE_REDIS_CONFIG_ERROR: EnvVar[bool] = env_var(False) @@ -763,8 +763,10 @@ class EnvironmentVariables: # Enables different behavior for when the backend would do a cold start if it was inactive. REFLEX_DOES_BACKEND_COLD_START: EnvVar[bool] = env_var(False) - # The timeout for the backend to do a cold start in seconds. - REFLEX_BACKEND_COLD_START_TIMEOUT: EnvVar[int] = env_var(10) + # The timeout for the backend to do a cold start. + REFLEX_BACKEND_COLD_START_TIMEOUT: EnvVar[timedelta] = env_var( + timedelta(seconds=10) + ) # Used by flexgen to enumerate the pages. REFLEX_ADD_ALL_ROUTES_ENDPOINT: EnvVar[bool] = env_var(False) @@ -777,11 +779,15 @@ class EnvironmentVariables: constants.POLLING_MAX_HTTP_BUFFER_SIZE ) - # The interval to send a ping to the websocket server in seconds. - REFLEX_SOCKET_INTERVAL: EnvVar[int] = env_var(constants.Ping.INTERVAL) + # The interval to send a ping to the websocket server. + REFLEX_SOCKET_INTERVAL: EnvVar[timedelta] = env_var( + timedelta(seconds=constants.Ping.INTERVAL) + ) - # The timeout to wait for a pong from the websocket server in seconds. - REFLEX_SOCKET_TIMEOUT: EnvVar[int] = env_var(constants.Ping.TIMEOUT) + # The timeout to wait for a pong from the websocket server. + REFLEX_SOCKET_TIMEOUT: EnvVar[timedelta] = env_var( + timedelta(seconds=constants.Ping.TIMEOUT) + ) # Whether to run Granian in a spawn process. This enables Reflex to pick up on environment variable changes between hot reloads. REFLEX_STRICT_HOT_RELOAD: EnvVar[bool] = env_var(False) @@ -826,9 +832,17 @@ class EnvironmentVariables: REFLEX_MOUNT_FRONTEND_COMPILED_APP: EnvVar[bool] = env_var(False, internal=True) # How long to delay writing updated states to disk. (Higher values mean less writes, but more chance of lost data.) + REFLEX_STATE_MANAGER_DISK_DEBOUNCE: EnvVar[timedelta] = env_var( + timedelta(seconds=2) + ) + + # Deprecated in favour of REFLEX_STATE_MANAGER_DISK_DEBOUNCE. REFLEX_STATE_MANAGER_DISK_DEBOUNCE_SECONDS: EnvVar[float] = env_var(2.0) # How long to wait between automatic reload on frontend error to avoid reload loops. + REFLEX_AUTO_RELOAD_COOLDOWN: EnvVar[timedelta] = env_var(timedelta(seconds=10)) + + # Deprecated in favour of REFLEX_AUTO_RELOAD_COOLDOWN. REFLEX_AUTO_RELOAD_COOLDOWN_TIME_MS: EnvVar[int] = env_var(10_000) # Whether to enable debug logging for the redis state manager. @@ -837,7 +851,10 @@ class EnvironmentVariables: # Whether to opportunistically hold the redis lock to allow fast in-memory access while uncontended. REFLEX_OPLOCK_ENABLED: EnvVar[bool] = env_var(False) - # How long to opportunistically hold the redis lock in milliseconds (must be less than the token expiration). + # How long to opportunistically hold the redis lock (must be less than the token expiration). + REFLEX_OPLOCK_HOLD_TIME: EnvVar[timedelta] = env_var(timedelta(0)) + + # Deprecated in favour of REFLEX_OPLOCK_HOLD_TIME. REFLEX_OPLOCK_HOLD_TIME_MS: EnvVar[int] = env_var(0) # Extra plugins to append to the config's plugins list. @@ -850,6 +867,96 @@ class EnvironmentVariables: environment = EnvironmentVariables() +# Superseded settings already warned about. A setting has no call site, so the +# per-location dedupe in `console.deprecate` would repeat the warning from every +# code path that reads it. +_WARNED_SUPERSEDED: set[str] = set() + + +def _duration_setting( + setting: EnvVar[timedelta], + superseded: EnvVar[int] | EnvVar[float], + unit: str, +) -> timedelta: + """Read a duration setting, honouring the unit-suffixed name it replaced. + + *superseded* carried its unit in its name and its value as a bare number, so it + cannot become a duration in place: ``10000`` would read as seconds rather than + the milliseconds it means. It is read in *unit* instead, and only while it is + still set. + + Args: + setting: The duration setting to read. + superseded: The setting it replaced, whose value is a count of *unit*. + unit: The suffix of the unit *superseded* counts in, as + :func:`interpret_timedelta_env` accepts it. + + Returns: + The configured duration. + """ + if not superseded.is_set(): + return setting.get() + + if superseded.name not in _WARNED_SUPERSEDED: + _WARNED_SUPERSEDED.add(superseded.name) + from reflex_base.utils import console + + # Spell out the exact replacement: renaming the variable without adding + # the unit would silently read its value as seconds. The number comes + # from the parsed value rather than the raw text, because the forms a + # bare int or float accepts are wider than the duration parser's: `.5`, + # `1_000` and `1e3` would all suggest a value that fails to parse. + replacement = f"{superseded.get()}{unit}" + console.deprecate( + feature_name=superseded.name, + reason=f"Set {setting.name}={replacement} instead.", + deprecation_version="0.9.12", + removal_version="1.0", + ) + if setting.is_set(): + return setting.get() + return superseded.get() * timedelta(**{_TIMEDELTA_UNITS[unit]: 1}) + + +def auto_reload_cooldown() -> timedelta: + """How long to wait between automatic reloads on a frontend error. + + Returns: + The configured duration. + """ + return _duration_setting( + environment.REFLEX_AUTO_RELOAD_COOLDOWN, + environment.REFLEX_AUTO_RELOAD_COOLDOWN_TIME_MS, + "ms", + ) + + +def oplock_hold_time() -> timedelta: + """How long to opportunistically hold the redis lock. + + Returns: + The configured duration. + """ + return _duration_setting( + environment.REFLEX_OPLOCK_HOLD_TIME, + environment.REFLEX_OPLOCK_HOLD_TIME_MS, + "ms", + ) + + +def state_manager_disk_debounce() -> timedelta: + """How long to delay writing updated states to disk. + + Returns: + The configured duration. + """ + return _duration_setting( + environment.REFLEX_STATE_MANAGER_DISK_DEBOUNCE, + environment.REFLEX_STATE_MANAGER_DISK_DEBOUNCE_SECONDS, + "s", + ) + + try: from dotenv import load_dotenv except ImportError: diff --git a/packages/reflex-base/src/reflex_base/utils/console.py b/packages/reflex-base/src/reflex_base/utils/console.py index 11541720ff6..9a82615f073 100644 --- a/packages/reflex-base/src/reflex_base/utils/console.py +++ b/packages/reflex-base/src/reflex_base/utils/console.py @@ -353,6 +353,15 @@ def _is_framework_filename(filename: str) -> bool: Returns: Whether the file lives under one of the excluded framework roots. """ + # Generated code carries a pseudo-name rather than a path: `` for + # `exec` and a dataclass's generated `__init__`, `` for the + # import machinery. Neither is a user call site, and treating one as a path + # would resolve it against the cwd, so whether it counted as framework code + # would depend on where the app was started from. Other bracketed names are + # left alone on purpose: `` and an `` cell are + # exactly where an interactive user would look for their own call. + if filename == "" or filename.startswith(" bool: Returns: Whether the file lives under one of the excluded framework roots. """ + # Generated code carries a pseudo-name rather than a path: `` for + # `exec` and a dataclass's generated `__init__`, `` for the + # import machinery. Neither is a user call site, and treating one as a path + # would resolve it against the cwd, so whether it counted as framework code + # would depend on where the app was started from. Other bracketed names are + # left alone on purpose: `` and an `` cell are + # exactly where an interactive user would look for their own call. + if filename == "" or filename.startswith(" list[str | Var]: if ({has_too_many_connection_errors!s}) {{ setWaitedForBackend(true); }} -}}, {environment.REFLEX_BACKEND_COLD_START_TIMEOUT.get() * 1000}); +}}, {environment.REFLEX_BACKEND_COLD_START_TIMEOUT.get() // timedelta(milliseconds=1)}); """ ) else: diff --git a/reflex/app.py b/reflex/app.py index 558a1d0f607..61c63210f44 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -32,7 +32,7 @@ from reflex_base.components.component import Component, ComponentStyle from reflex_base.config import get_config, reload_config from reflex_base.context.base import BaseContext -from reflex_base.environment import environment +from reflex_base.environment import auto_reload_cooldown, environment from reflex_base.event import ( _EVENT_FIELDS, Event, @@ -585,6 +585,10 @@ def _setup_state(self) -> None: # Set up the state manager. self._state_manager = StateManager.create() + # Read the auto-reload cooldown now so a deprecated name warns at startup + # rather than on the first frontend error that consults it. + auto_reload_cooldown() + # Set up the Socket.IO AsyncServer. if not self.sio: self.sio = AsyncServer( @@ -600,8 +604,8 @@ def _setup_state(self) -> None: ), cors_credentials=config.transport == "websocket", max_http_buffer_size=environment.REFLEX_SOCKET_MAX_HTTP_BUFFER_SIZE.get(), - ping_interval=environment.REFLEX_SOCKET_INTERVAL.get(), - ping_timeout=environment.REFLEX_SOCKET_TIMEOUT.get(), + ping_interval=environment.REFLEX_SOCKET_INTERVAL.get().total_seconds(), + ping_timeout=environment.REFLEX_SOCKET_TIMEOUT.get().total_seconds(), json=SimpleNamespace( dumps=staticmethod(_sio_dumps), loads=staticmethod(_sio_loads), diff --git a/reflex/istate/manager/disk.py b/reflex/istate/manager/disk.py index b1da40ae209..ffd00dd5ec7 100644 --- a/reflex/istate/manager/disk.py +++ b/reflex/istate/manager/disk.py @@ -11,7 +11,7 @@ from pathlib import Path from typing import Any, Generic, cast -from reflex_base.environment import environment +from reflex_base.environment import state_manager_disk_debounce from typing_extensions import Unpack, override from reflex.istate.manager import ( @@ -68,7 +68,7 @@ class StateManagerDisk(StateManager): ) _write_queue_task: asyncio.Task | None = None _write_debounce_seconds: float = dataclasses.field( - default=environment.REFLEX_STATE_MANAGER_DISK_DEBOUNCE_SECONDS.get() + default_factory=lambda: state_manager_disk_debounce().total_seconds() ) def __post_init__(self): diff --git a/reflex/istate/manager/redis.py b/reflex/istate/manager/redis.py index 4255b5f0140..783dbb77f74 100644 --- a/reflex/istate/manager/redis.py +++ b/reflex/istate/manager/redis.py @@ -10,13 +10,15 @@ import time import uuid from collections.abc import AsyncIterator +from datetime import timedelta from typing import Any, TypedDict, cast from redis import ResponseError from redis.asyncio import Redis from reflex_base.config import get_config -from reflex_base.environment import environment +from reflex_base.environment import environment, oplock_hold_time from reflex_base.utils.exceptions import ( + EnvironmentVarValueError, InvalidLockWarningThresholdError, LockExpiredError, StateSchemaMismatchError, @@ -87,10 +89,22 @@ def _default_oplock_hold_time_ms() -> int: Returns: The default opportunistic lock hold time. + + Raises: + EnvironmentVarValueError: If the configured hold time is negative. """ - return environment.REFLEX_OPLOCK_HOLD_TIME_MS.get() or ( - _default_lock_expiration() // 2 - ) + hold_time = oplock_hold_time() + if hold_time < timedelta(0): + msg = ( + "The opportunistic lock hold time must not be negative, got " + f"{hold_time.total_seconds()} seconds." + ) + raise EnvironmentVarValueError(msg) + if not hold_time: + return _default_lock_expiration() // 2 + # A configured hold time is worth at least one millisecond, so that a + # sub-millisecond duration is not mistaken for the unset default above. + return max(hold_time // timedelta(milliseconds=1), 1) # The lock waiter task should subscribe to lock channel updates within this period. diff --git a/reflex/model.py b/reflex/model.py index dbe0e460ef1..67fa3065874 100644 --- a/reflex/model.py +++ b/reflex/model.py @@ -89,7 +89,7 @@ def get_engine_args(url: str | None = None) -> dict[str, Any]: "pool_size": environment.SQLALCHEMY_POOL_SIZE.get(), "max_overflow": environment.SQLALCHEMY_MAX_OVERFLOW.get(), "pool_recycle": environment.SQLALCHEMY_POOL_RECYCLE.get(), - "pool_timeout": environment.SQLALCHEMY_POOL_TIMEOUT.get(), + "pool_timeout": environment.SQLALCHEMY_POOL_TIMEOUT.get().total_seconds(), } conf = get_config() url = url or conf.db_url diff --git a/reflex/state.py b/reflex/state.py index 1ea080cc863..e0adde54fd4 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -15,6 +15,7 @@ import sys import time from collections.abc import Callable, Iterator, Mapping, Sequence +from datetime import timedelta from hashlib import md5 from types import FunctionType from typing import ( @@ -30,7 +31,7 @@ from reflex_base import constants from reflex_base.constants.state import FIELD_MARKER -from reflex_base.environment import PerformanceMode, environment +from reflex_base.environment import PerformanceMode, auto_reload_cooldown, environment from reflex_base.event import ( EVENT_ACTIONS_MARKER, Event, @@ -2563,7 +2564,7 @@ def handle_frontend_exception( ): yield call_script( f"const last_reload = parseInt(window.sessionStorage.getItem('{LAST_RELOADED_KEY}')) || 0;" - f"if (Date.now() - last_reload > {environment.REFLEX_AUTO_RELOAD_COOLDOWN_TIME_MS.get()})" + f"if (Date.now() - last_reload > {auto_reload_cooldown() // timedelta(milliseconds=1)})" "{" f"window.sessionStorage.setItem('{LAST_RELOADED_KEY}', Date.now().toString());" "window.location.reload();" diff --git a/tests/units/istate/manager/test_redis.py b/tests/units/istate/manager/test_redis.py index 3a336501b84..e382364e9b8 100644 --- a/tests/units/istate/manager/test_redis.py +++ b/tests/units/istate/manager/test_redis.py @@ -9,8 +9,13 @@ import pytest import pytest_asyncio +from reflex_base.utils.exceptions import EnvironmentVarValueError -from reflex.istate.manager.redis import StateManagerRedis +from reflex.istate.manager.redis import ( + StateManagerRedis, + _default_lock_expiration, + _default_oplock_hold_time_ms, +) from reflex.istate.manager.token import BaseStateToken from reflex.state import BaseState from tests.units.mock_redis import mock_redis, real_redis @@ -759,3 +764,33 @@ async def modify(): ) assert isinstance(final_state, root_state) assert final_state.count == 2 + + +def test_oplock_hold_time_below_one_millisecond( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A sub-millisecond hold time must not read as the unset default. + + Zero means "use half the lock expiration", so a duration that floors to + zero milliseconds has to round up instead of falling into that branch. + """ + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME", "500us") + assert _default_oplock_hold_time_ms() == 1 + + +def test_oplock_hold_time_unset_halves_the_lock_expiration( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unset hold time keeps deriving from the lock expiration.""" + monkeypatch.delenv("REFLEX_OPLOCK_HOLD_TIME", raising=False) + monkeypatch.delenv("REFLEX_OPLOCK_HOLD_TIME_MS", raising=False) + assert _default_oplock_hold_time_ms() == _default_lock_expiration() // 2 + + +def test_oplock_hold_time_rejects_a_negative_duration( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A negative hold time is a configuration error, not one millisecond.""" + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME", "-5s") + with pytest.raises(EnvironmentVarValueError, match="must not be negative"): + _default_oplock_hold_time_ms() diff --git a/tests/units/reflex_base/utils/test_log.py b/tests/units/reflex_base/utils/test_log.py index e90c25a7b47..7daf8edd4b4 100644 --- a/tests/units/reflex_base/utils/test_log.py +++ b/tests/units/reflex_base/utils/test_log.py @@ -279,6 +279,61 @@ def test_deprecate_dedupes_and_renders(capsys): assert "removed in 1.0" in out +@pytest.mark.parametrize( + "filename", + [ + "", + "", + "", + ], +) +def test_generated_code_is_never_a_user_call_site(filename: str): + """Code the interpreter generated has a pseudo-name, not a path. + + Args: + filename: The pseudo-filename the generated code object carries. + """ + assert log._is_framework_filename(filename) + + +@pytest.mark.parametrize("filename", ["", ""]) +def test_an_interactive_call_site_is_still_reported(filename: str): + """A REPL line and a notebook cell are the user's own code. + + They are bracketed like generated code but are exactly the location a + deprecation should name, so the rule must not swallow them. + + Args: + filename: The pseudo-filename an interactive session carries. + """ + assert not log._is_framework_filename(filename) + + +def test_an_ordinary_path_is_still_classified_by_location(tmp_path): + """The rule must not swallow a real file outside the framework. + + Args: + tmp_path: pytest temporary directory fixture. + """ + assert not log._is_framework_filename(str(tmp_path / "app.py")) + + +def test_deprecate_skips_frames_compiled_from_strings(capsys): + """A `` code object is not a user call site, so the location skips it.""" + namespace: dict[str, object] = {} + exec( + "def emit():\n" + " log.deprecate(feature_name='StringFeature', reason='Use x.'," + " deprecation_version='0.1.0', removal_version='1.0')\n", + {"log": log}, + namespace, + ) + namespace["emit"]() # pyright: ignore[reportCallIssue] + out, _ = capsys.readouterr() + assert "" not in out + assert "test_log.py" in out + + def test_deprecate_json_extras(monkeypatch, capsys): """Deprecations carry structured metadata in JSON mode.""" monkeypatch.setenv("REFLEX_LOG_JSON", "true") diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 8917ef90838..1962fe32671 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -243,6 +243,28 @@ def logout(self): # pyright: ignore [reportIncompatibleMethodOverride] return TestAuthProvider +def test_app_warns_about_a_deprecated_duration_name_at_startup( + monkeypatch: pytest.MonkeyPatch, +): + """The auto-reload cooldown is only consulted on a frontend error. + + Reading it while the app is set up surfaces the deprecation when the app + starts, where a developer will actually see it. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.delenv("REFLEX_AUTO_RELOAD_COOLDOWN", raising=False) + monkeypatch.setenv("REFLEX_AUTO_RELOAD_COOLDOWN_TIME_MS", "5000") + monkeypatch.setattr("reflex_base.environment._WARNED_SUPERSEDED", set()) + + with unittest.mock.patch("reflex_base.utils.console.deprecate") as deprecate: + App(_state=EmptyState) + + feature_names = [call.kwargs["feature_name"] for call in deprecate.call_args_list] + assert "REFLEX_AUTO_RELOAD_COOLDOWN_TIME_MS" in feature_names + + def test_default_app(app: App): """Test creating an app with no args. diff --git a/tests/units/test_environment.py b/tests/units/test_environment.py index f6e85efca7a..bd4a747cf41 100644 --- a/tests/units/test_environment.py +++ b/tests/units/test_environment.py @@ -34,6 +34,8 @@ interpret_plugin_class_env, interpret_plugin_env, interpret_timedelta_env, + oplock_hold_time, + state_manager_disk_debounce, ) from reflex_base.plugins import Plugin from reflex_base.utils.exceptions import EnvironmentVarValueError @@ -791,3 +793,190 @@ def test_timedelta_env_var_round_trips_through_set( # never matches - the same quirk the other `set` tests here work around. env_var_instance.set(value) # type: ignore[arg-type] assert env_var_instance.get() == value + + +@pytest.fixture(autouse=True) +def _forget_superseded_warnings(monkeypatch: pytest.MonkeyPatch) -> None: + """Each test sees the deprecation warning as if the process had just started. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.setattr("reflex_base.environment._WARNED_SUPERSEDED", set()) + + +def test_a_superseded_duration_setting_warns_once_per_process( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Every code path that reads the setting must not repeat the warning. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.delenv("REFLEX_OPLOCK_HOLD_TIME", raising=False) + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME_MS", "250") + + with patch("reflex_base.utils.console.deprecate") as deprecate: + oplock_hold_time() + oplock_hold_time() + + deprecate.assert_called_once() + + +def test_duration_env_vars_accept_a_suffix(monkeypatch: pytest.MonkeyPatch) -> None: + """The duration settings read a unit, not just a count of seconds. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.setenv("REFLEX_SOCKET_TIMEOUT", "2m") + + assert environment.REFLEX_SOCKET_TIMEOUT.get() == timedelta(minutes=2) + + +def test_duration_env_vars_still_read_a_bare_number_as_seconds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Values set before these became durations keep their meaning. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.setenv("SQLALCHEMY_POOL_TIMEOUT", "45") + + assert environment.SQLALCHEMY_POOL_TIMEOUT.get() == timedelta(seconds=45) + + +def test_a_superseded_duration_setting_is_still_honoured( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Its value counts the unit in its name, not seconds. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.delenv("REFLEX_OPLOCK_HOLD_TIME", raising=False) + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME_MS", "250") + + with patch("reflex_base.utils.console.deprecate") as deprecate: + assert oplock_hold_time() == timedelta(milliseconds=250) + + deprecate.assert_called_once() + assert deprecate.call_args.kwargs["feature_name"] == "REFLEX_OPLOCK_HOLD_TIME_MS" + # The exact replacement, so nobody renames the variable and loses the unit. + assert "REFLEX_OPLOCK_HOLD_TIME=250ms" in deprecate.call_args.kwargs["reason"] + + +def test_a_superseded_seconds_setting_names_its_replacement_in_seconds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A fractional count of seconds converts to the matching suffixed value. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.delenv("REFLEX_STATE_MANAGER_DISK_DEBOUNCE", raising=False) + monkeypatch.setenv("REFLEX_STATE_MANAGER_DISK_DEBOUNCE_SECONDS", "0.5") + + with patch("reflex_base.utils.console.deprecate") as deprecate: + assert state_manager_disk_debounce() == timedelta(milliseconds=500) + + assert ( + "REFLEX_STATE_MANAGER_DISK_DEBOUNCE=0.5s" + in deprecate.call_args.kwargs["reason"] + ) + + +@pytest.mark.parametrize( + ("superseded_value", "suggested"), + [(".5", "0.5s"), ("1e3", "1000.0s"), ("2.0", "2.0s")], +) +def test_the_suggested_replacement_always_parses( + monkeypatch: pytest.MonkeyPatch, superseded_value: str, suggested: str +) -> None: + """A bare float accepts forms the duration parser rejects. + + Suggesting the raw text would tell a project to set a value that fails to + start the app, so the number comes from the parsed value instead. + + Args: + monkeypatch: pytest monkeypatch fixture. + superseded_value: The value set on the superseded setting. + suggested: The replacement the warning should name. + """ + monkeypatch.delenv("REFLEX_STATE_MANAGER_DISK_DEBOUNCE", raising=False) + monkeypatch.setenv("REFLEX_STATE_MANAGER_DISK_DEBOUNCE_SECONDS", superseded_value) + + with patch("reflex_base.utils.console.deprecate") as deprecate: + state_manager_disk_debounce() + + reason = deprecate.call_args.kwargs["reason"] + assert f"REFLEX_STATE_MANAGER_DISK_DEBOUNCE={suggested}" in reason + # The suggestion has to survive being pasted back into the environment. + assert interpret_timedelta_env(suggested, "REFLEX_STATE_MANAGER_DISK_DEBOUNCE") + + +def test_a_blank_superseded_setting_counts_as_unset( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A blank value is unset everywhere else in the environment, so no warning. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.delenv("REFLEX_OPLOCK_HOLD_TIME", raising=False) + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME_MS", " ") + + with patch("reflex_base.utils.console.deprecate") as deprecate: + assert oplock_hold_time() == timedelta(0) + + deprecate.assert_not_called() + + +def test_a_blank_duration_setting_does_not_shadow_the_one_it_supersedes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Templated deployments leave the new name present but empty. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME", "") + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME_MS", "250") + + with patch("reflex_base.utils.console.deprecate"): + assert oplock_hold_time() == timedelta(milliseconds=250) + + +def test_the_duration_setting_wins_over_the_one_it_supersedes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Setting both is how a project migrates, so the new name has to lead. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME_MS", "250") + monkeypatch.setenv("REFLEX_OPLOCK_HOLD_TIME", "2s") + + with patch("reflex_base.utils.console.deprecate") as deprecate: + assert oplock_hold_time() == timedelta(seconds=2) + + deprecate.assert_called_once() + + +def test_a_duration_setting_left_alone_does_not_warn( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The deprecation only fires for projects that actually set the old name. + + Args: + monkeypatch: pytest monkeypatch fixture. + """ + monkeypatch.delenv("REFLEX_OPLOCK_HOLD_TIME_MS", raising=False) + monkeypatch.delenv("REFLEX_OPLOCK_HOLD_TIME", raising=False) + + with patch("reflex_base.utils.console.deprecate") as deprecate: + assert oplock_hold_time() == timedelta(0) + + deprecate.assert_not_called()