From 9bbb04ecfc51cd97d26fd8929644592229340a46 Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Wed, 6 Nov 2024 00:15:04 +0100 Subject: [PATCH 01/11] implement redis key prefix for StateManagerRedis --- reflex/config.py | 3 +++ reflex/state.py | 31 +++++++++++++++++++++++++----- tests/units/test_state.py | 35 ++++++++++++++++++++++++++++++---- tests/units/test_state_tree.py | 7 ++++++- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/reflex/config.py b/reflex/config.py index 049cc2e83..a57dfd731 100644 --- a/reflex/config.py +++ b/reflex/config.py @@ -545,6 +545,9 @@ class EnvironmentVariables: # Where to save screenshots when tests fail. SCREENSHOT_DIR: EnvVar[Optional[Path]] = env_var(None) + # Optional redis key prefix for the state manager. + REFLEX_REDIS_PREFIX: EnvVar[Optional[str]] = env_var(None) + environment = EnvironmentVariables() diff --git a/reflex/state.py b/reflex/state.py index a53df7b6f..9fbd370b8 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -42,7 +42,7 @@ from sqlalchemy.orm import DeclarativeBase from typing_extensions import Self from reflex import event -from reflex.config import get_config +from reflex.config import EnvironmentVariables, get_config from reflex.istate.data import RouterData from reflex.istate.storage import ( ClientStorageBase, @@ -3076,6 +3076,26 @@ def _default_lock_expiration() -> int: return get_config().redis_lock_expiration +TOKEN_TYPE = TypeVar("TOKEN_TYPE", str, bytes) + + +def prefix_redis_token(token: TOKEN_TYPE) -> TOKEN_TYPE: + """Prefix the token with the redis prefix. + + Args: + token: The token to prefix. + + Returns: + The prefixed token. + """ + prefix = EnvironmentVariables.REFLEX_REDIS_PREFIX.get() + if not prefix: + return token + if isinstance(token, bytes): + return prefix.encode() + token + return f"{prefix}{token}" + + class StateManagerRedis(StateManager): """A state manager that stores states in redis.""" @@ -3213,7 +3233,7 @@ class StateManagerRedis(StateManager): state = None # Fetch the serialized substate from redis. - redis_state = await self.redis.get(token) + redis_state = await self.redis.get(prefix_redis_token(token)) if redis_state is not None: # Deserialize the substate. @@ -3268,7 +3288,8 @@ class StateManagerRedis(StateManager): # Check that we're holding the lock. if ( lock_id is not None - and await self.redis.get(self._lock_key(token)) != lock_id + and await self.redis.get(prefix_redis_token(self._lock_key(token))) + != lock_id ): raise LockExpiredError( f"Lock expired for token {token} while processing. Consider increasing " @@ -3299,7 +3320,7 @@ class StateManagerRedis(StateManager): pickle_state = state._serialize() if pickle_state: await self.redis.set( - _substate_key(client_token, state), + prefix_redis_token(_substate_key(client_token, state)), pickle_state, ex=self.token_expiration, ) @@ -3349,7 +3370,7 @@ class StateManagerRedis(StateManager): True if the lock was obtained. """ return await self.redis.set( - lock_key, + prefix_redis_token(lock_key), lock_id, px=self.lock_expiration, nx=True, # only set if it doesn't exist diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 83e348cd2..685b0ccfc 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -40,6 +40,7 @@ from reflex.state import ( StateProxy, StateUpdate, _substate_key, + prefix_redis_token, ) from reflex.testing import chdir from reflex.utils import format, prerequisites, types @@ -1671,7 +1672,7 @@ async def test_state_manager_modify_state( """ async with state_manager.modify_state(substate_token) as state: if isinstance(state_manager, StateManagerRedis): - assert await state_manager.redis.get(f"{token}_lock") + assert await state_manager.redis.get(prefix_redis_token(f"{token}_lock")) elif isinstance(state_manager, (StateManagerMemory, StateManagerDisk)): assert token in state_manager._states_locks assert state_manager._states_locks[token].locked() @@ -1681,7 +1682,9 @@ async def test_state_manager_modify_state( state.complex[3] = complex_1 # lock should be dropped after exiting the context if isinstance(state_manager, StateManagerRedis): - assert (await state_manager.redis.get(f"{token}_lock")) is None + assert ( + await state_manager.redis.get(prefix_redis_token(f"{token}_lock")) + ) is None elif isinstance(state_manager, (StateManagerMemory, StateManagerDisk)): assert not state_manager._states_locks[token].locked() @@ -1723,7 +1726,9 @@ async def test_state_manager_contend( assert (await state_manager.get_state(substate_token)).num1 == exp_num1 if isinstance(state_manager, StateManagerRedis): - assert (await state_manager.redis.get(f"{token}_lock")) is None + assert ( + await state_manager.redis.get(prefix_redis_token(f"{token}_lock")) + ) is None elif isinstance(state_manager, (StateManagerMemory, StateManagerDisk)): assert token in state_manager._states_locks assert not state_manager._states_locks[token].locked() @@ -1783,7 +1788,7 @@ async def test_state_manager_lock_expire( @pytest.mark.asyncio async def test_state_manager_lock_expire_contend( - state_manager_redis: StateManager, token: str, substate_token_redis: str + state_manager_redis: StateManagerRedis, token: str, substate_token_redis: str ): """Test that the state manager lock expires and queued waiters proceed. @@ -1825,6 +1830,28 @@ async def test_state_manager_lock_expire_contend( assert (await state_manager_redis.get_state(substate_token_redis)).num1 == exp_num1 +@pytest.mark.asyncio +async def test_state_manager_redis_prefix( + state_manager_redis: StateManagerRedis, substate_token_redis: str +): + """Test that the state manager redis prefix is applied correctly. + + Args: + state_manager_redis: A state manager instance. + substate_token_redis: A token + substate name for looking up in state manager. + """ + prefix = "test_prefix" + reflex.config.EnvironmentVariables.REFLEX_REDIS_PREFIX.set(prefix) + + async with state_manager_redis.modify_state(substate_token_redis) as state: + state.num1 = 42 + + prefixed_token = prefix_redis_token(substate_token_redis) + assert prefixed_token == f"{prefix}{substate_token_redis}" + + assert await state_manager_redis.redis.get(prefixed_token) + + @pytest.fixture(scope="function") def mock_app(monkeypatch, state_manager: StateManager) -> rx.App: """Mock app fixture. diff --git a/tests/units/test_state_tree.py b/tests/units/test_state_tree.py index ebdd877de..aa853af9b 100644 --- a/tests/units/test_state_tree.py +++ b/tests/units/test_state_tree.py @@ -6,7 +6,12 @@ import pytest import pytest_asyncio import reflex as rx -from reflex.state import BaseState, StateManager, StateManagerRedis, _substate_key +from reflex.state import ( + BaseState, + StateManager, + StateManagerRedis, + _substate_key, +) class Root(BaseState): From 211d4b46676910be3aef3b875c2dfdad4713640a Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Wed, 6 Nov 2024 21:34:14 +0100 Subject: [PATCH 02/11] yielding redis prefix fixture --- tests/units/test_state.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 685b0ccfc..4cfa68254 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -10,7 +10,7 @@ import os import sys import threading from textwrap import dedent -from typing import Any, AsyncGenerator, Callable, Dict, List, Optional, Union +from typing import Any, AsyncGenerator, Callable, Dict, Generator, List, Optional, Union from unittest.mock import AsyncMock, Mock import pytest @@ -1830,24 +1830,37 @@ async def test_state_manager_lock_expire_contend( assert (await state_manager_redis.get_state(substate_token_redis)).num1 == exp_num1 +@pytest.fixture(scope="function") +def redis_prefix() -> Generator[str, None, None]: + """Fixture for redis prefix. + + Yields: + A redis prefix. + """ + prefix = "test_prefix" + reflex.config.EnvironmentVariables.REFLEX_REDIS_PREFIX.set(prefix) + yield prefix + reflex.config.EnvironmentVariables.REFLEX_REDIS_PREFIX.set(None) + + @pytest.mark.asyncio async def test_state_manager_redis_prefix( - state_manager_redis: StateManagerRedis, substate_token_redis: str + state_manager_redis: StateManagerRedis, + substate_token_redis: str, + redis_prefix: str, ): """Test that the state manager redis prefix is applied correctly. Args: state_manager_redis: A state manager instance. substate_token_redis: A token + substate name for looking up in state manager. + redis_prefix: A redis prefix. """ - prefix = "test_prefix" - reflex.config.EnvironmentVariables.REFLEX_REDIS_PREFIX.set(prefix) - async with state_manager_redis.modify_state(substate_token_redis) as state: state.num1 = 42 prefixed_token = prefix_redis_token(substate_token_redis) - assert prefixed_token == f"{prefix}{substate_token_redis}" + assert prefixed_token == f"{redis_prefix}{substate_token_redis}" assert await state_manager_redis.redis.get(prefixed_token) From 8c38f773e0fc7886634292086420b47bee23f02f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Brand=C3=A9ho?= Date: Wed, 6 Nov 2024 13:32:31 -0800 Subject: [PATCH 03/11] stop ignoring some lint rules (#4311) * bump python packages version * stop ignoring some lint rules that pass ruff check * stop ignoring rule F541 * remove sneaky test file --- benchmarks/test_benchmark_compile_pages.py | 10 ++--- pyproject.toml | 2 +- reflex/components/component.py | 1 - reflex/components/core/banner.py | 2 +- .../datadisplay/shiki_code_block.py | 37 +++++++++---------- reflex/components/lucide/icon.py | 2 +- .../components/radix/primitives/accordion.py | 6 +-- reflex/components/radix/themes/base.py | 2 +- reflex/constants/base.py | 2 +- reflex/custom_components/custom_components.py | 8 ++-- reflex/reflex.py | 2 +- reflex/utils/console.py | 1 - reflex/utils/exec.py | 2 +- tests/integration/test_background_task.py | 2 +- tests/integration/test_computed_vars.py | 2 +- tests/integration/test_dynamic_routes.py | 2 +- tests/integration/test_event_actions.py | 2 +- tests/integration/test_large_state.py | 2 +- tests/integration/test_navigation.py | 2 +- tests/integration/test_server_side_event.py | 1 - tests/integration/test_state_inheritance.py | 2 +- tests/integration/test_upload.py | 4 +- tests/units/compiler/test_compiler.py | 10 ++--- tests/units/components/core/test_debounce.py | 1 - tests/units/components/test_component.py | 2 +- tests/units/test_app.py | 4 +- tests/units/test_state.py | 2 +- tests/units/test_var.py | 2 +- tests/units/utils/test_serializers.py | 3 -- 29 files changed, 56 insertions(+), 64 deletions(-) diff --git a/benchmarks/test_benchmark_compile_pages.py b/benchmarks/test_benchmark_compile_pages.py index 4448bca45..f232eb627 100644 --- a/benchmarks/test_benchmark_compile_pages.py +++ b/benchmarks/test_benchmark_compile_pages.py @@ -210,7 +210,7 @@ def app_with_one_page( Yields: an AppHarness instance """ - root = tmp_path_factory.mktemp(f"app1") + root = tmp_path_factory.mktemp("app1") yield AppHarness.create(root=root, app_source=AppWithOnePage) # type: ignore @@ -227,7 +227,7 @@ def app_with_ten_pages( Yields: an AppHarness instance """ - root = tmp_path_factory.mktemp(f"app10") + root = tmp_path_factory.mktemp("app10") yield AppHarness.create( root=root, app_source=functools.partial( @@ -249,7 +249,7 @@ def app_with_hundred_pages( Yields: an AppHarness instance """ - root = tmp_path_factory.mktemp(f"app100") + root = tmp_path_factory.mktemp("app100") yield AppHarness.create( root=root, @@ -272,7 +272,7 @@ def app_with_thousand_pages( Yields: an AppHarness instance """ - root = tmp_path_factory.mktemp(f"app1000") + root = tmp_path_factory.mktemp("app1000") yield AppHarness.create( root=root, @@ -295,7 +295,7 @@ def app_with_ten_thousand_pages( Yields: running AppHarness instance """ - root = tmp_path_factory.mktemp(f"app10000") + root = tmp_path_factory.mktemp("app10000") yield AppHarness.create( root=root, diff --git a/pyproject.toml b/pyproject.toml index a3e3a17f6..585d4703f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -92,7 +92,7 @@ build-backend = "poetry.core.masonry.api" [tool.ruff] target-version = "py39" lint.select = ["B", "D", "E", "F", "I", "SIM", "W"] -lint.ignore = ["B008", "D203", "D205", "D213", "D401", "D406", "D407", "E501", "F403", "F405", "F541", "SIM115"] +lint.ignore = ["B008", "D205", "E501", "F403", "SIM115"] lint.pydocstyle.convention = "google" [tool.ruff.lint.per-file-ignores] diff --git a/reflex/components/component.py b/reflex/components/component.py index 9a9e49a9d..2b934277b 100644 --- a/reflex/components/component.py +++ b/reflex/components/component.py @@ -158,7 +158,6 @@ class ComponentNamespace(SimpleNamespace): def __hash__(self) -> int: """Get the hash of the namespace. - Returns: The hash of the namespace. """ diff --git a/reflex/components/core/banner.py b/reflex/components/core/banner.py index 8a37b0bf7..f6abbab90 100644 --- a/reflex/components/core/banner.py +++ b/reflex/components/core/banner.py @@ -110,7 +110,7 @@ class ConnectionToaster(Toaster): individual_hooks = [ f"const toast_props = {str(LiteralVar.create(props))};", - f"const [userDismissed, setUserDismissed] = useState(false);", + "const [userDismissed, setUserDismissed] = useState(false);", FunctionStringVar( "useEffect", _var_data=VarData( diff --git a/reflex/components/datadisplay/shiki_code_block.py b/reflex/components/datadisplay/shiki_code_block.py index 21f22424d..4a3e05d0e 100644 --- a/reflex/components/datadisplay/shiki_code_block.py +++ b/reflex/components/datadisplay/shiki_code_block.py @@ -26,49 +26,48 @@ from reflex.vars.sequence import StringVar, string_replace_operation def copy_script() -> Any: """Copy script for the code block and modify the child SVG element. - Returns: Any: The result of calling the script. """ return run_script( - f""" + """ // Event listener for the parent click -document.addEventListener('click', function(event) {{ +document.addEventListener('click', function(event) { // Find the closest button (parent element) const parent = event.target.closest('button'); // If the parent is found - if (parent) {{ + if (parent) { // Find the SVG element within the parent const svgIcon = parent.querySelector('svg'); // If the SVG exists, proceed with the script - if (svgIcon) {{ + if (svgIcon) { const originalPath = svgIcon.innerHTML; const checkmarkPath = ''; // Checkmark SVG path - function transition(element, scale, opacity) {{ - element.style.transform = `scale(${{scale}})`; + function transition(element, scale, opacity) { + element.style.transform = `scale(${scale})`; element.style.opacity = opacity; - }} + } // Animate the SVG transition(svgIcon, 0, '0'); - setTimeout(() => {{ + setTimeout(() => { svgIcon.innerHTML = checkmarkPath; // Replace content with checkmark svgIcon.setAttribute('viewBox', '0 0 24 24'); // Adjust viewBox if necessary transition(svgIcon, 1, '1'); - setTimeout(() => {{ + setTimeout(() => { transition(svgIcon, 0, '0'); - setTimeout(() => {{ + setTimeout(() => { svgIcon.innerHTML = originalPath; // Restore original SVG content transition(svgIcon, 1, '1'); - }}, 125); - }}, 600); - }}, 125); - }} else {{ + }, 125); + }, 600); + }, 125); + } else { // console.error('SVG element not found within the parent.'); - }} - }} else {{ + } + } else { // console.error('Parent element not found.'); - }} -}}) + } +}) """ ) diff --git a/reflex/components/lucide/icon.py b/reflex/components/lucide/icon.py index 1ee68aaa3..b32fb8de3 100644 --- a/reflex/components/lucide/icon.py +++ b/reflex/components/lucide/icon.py @@ -58,7 +58,7 @@ class Icon(LucideIconComponent): props["tag"] = format.to_title_case(format.to_snake_case(props["tag"])) + "Icon" props["alias"] = f"Lucide{props['tag']}" - props.setdefault("color", f"var(--current-color)") + props.setdefault("color", "var(--current-color)") return super().create(*children, **props) diff --git a/reflex/components/radix/primitives/accordion.py b/reflex/components/radix/primitives/accordion.py index 272274723..608fee69d 100644 --- a/reflex/components/radix/primitives/accordion.py +++ b/reflex/components/radix/primitives/accordion.py @@ -382,7 +382,7 @@ class AccordionTrigger(AccordionComponent): "background_color": color("accent", 4), }, "& > .AccordionChevron": { - "transition": f"transform var(--animation-duration) var(--animation-easing)", + "transition": "transform var(--animation-duration) var(--animation-easing)", }, _inherited_variant_selector("classic"): { "color": "var(--accent-contrast)", @@ -485,11 +485,11 @@ to { The style of the component. """ slideDown = LiteralVar.create( - f"${{slideDown}} var(--animation-duration) var(--animation-easing)", + "${slideDown} var(--animation-duration) var(--animation-easing)", ) slideUp = LiteralVar.create( - f"${{slideUp}} var(--animation-duration) var(--animation-easing)", + "${slideUp} var(--animation-duration) var(--animation-easing)", ) return { diff --git a/reflex/components/radix/themes/base.py b/reflex/components/radix/themes/base.py index acca1dce8..20ebdc5a6 100644 --- a/reflex/components/radix/themes/base.py +++ b/reflex/components/radix/themes/base.py @@ -236,7 +236,7 @@ class Theme(RadixThemesComponent): tag = super()._render(props) tag.add_props( css=Var( - _js_expr=f"{{...theme.styles.global[':root'], ...theme.styles.global.body}}" + _js_expr="{...theme.styles.global[':root'], ...theme.styles.global.body}" ), ) return tag diff --git a/reflex/constants/base.py b/reflex/constants/base.py index 5042b453e..798ac7dc6 100644 --- a/reflex/constants/base.py +++ b/reflex/constants/base.py @@ -78,7 +78,7 @@ class Reflex(SimpleNamespace): # The root directory of the reflex library. ROOT_DIR = Path(__file__).parents[2] - RELEASES_URL = f"https://api.github.com/repos/reflex-dev/templates/releases" + RELEASES_URL = "https://api.github.com/repos/reflex-dev/templates/releases" class ReflexHostingCLI(SimpleNamespace): diff --git a/reflex/custom_components/custom_components.py b/reflex/custom_components/custom_components.py index 7681caebc..6be64ae2d 100644 --- a/reflex/custom_components/custom_components.py +++ b/reflex/custom_components/custom_components.py @@ -779,8 +779,8 @@ def _validate_project_info(): ) # PyPI only shows the first author. author = project.get("authors", [{}])[0] - author["name"] = console.ask(f"Author Name", default=author.get("name", "")) - author["email"] = console.ask(f"Author Email", default=author.get("email", "")) + author["name"] = console.ask("Author Name", default=author.get("name", "")) + author["email"] = console.ask("Author Email", default=author.get("email", "")) console.print(f'Current keywords are: {project.get("keywords") or []}') keyword_action = console.ask( @@ -923,7 +923,7 @@ def _get_file_from_prompt_in_loop() -> Tuple[bytes, str] | None: image_file = file_extension = None while image_file is None: image_filepath = console.ask( - f"Upload a preview image of your demo app (enter to skip)" + "Upload a preview image of your demo app (enter to skip)" ) if not image_filepath: break @@ -973,6 +973,6 @@ def install( console.set_log_level(loglevel) if _pip_install_on_demand(package_name=".", install_args=["-e"]): - console.info(f"Package installed successfully!") + console.info("Package installed successfully!") else: raise typer.Exit(code=1) diff --git a/reflex/reflex.py b/reflex/reflex.py index 6ccba01d3..8c5f0fc6f 100644 --- a/reflex/reflex.py +++ b/reflex/reflex.py @@ -363,7 +363,7 @@ def _login() -> str: access_token = hosting.authenticate_on_browser(invitation_code) if not access_token: - console.error(f"Unable to authenticate. Please try again or contact support.") + console.error("Unable to authenticate. Please try again or contact support.") raise typer.Exit(1) console.print("Successfully logged in.") diff --git a/reflex/utils/console.py b/reflex/utils/console.py index 20c699e20..04e590910 100644 --- a/reflex/utils/console.py +++ b/reflex/utils/console.py @@ -191,7 +191,6 @@ def ask( def progress(): """Create a new progress bar. - Returns: A new progress bar. """ diff --git a/reflex/utils/exec.py b/reflex/utils/exec.py index fb613810a..5291de095 100644 --- a/reflex/utils/exec.py +++ b/reflex/utils/exec.py @@ -429,7 +429,7 @@ def output_system_info(): except Exception: config_file = None - console.rule(f"System Info") + console.rule("System Info") console.debug(f"Config file: {config_file!r}") console.debug(f"Config: {config}") diff --git a/tests/integration/test_background_task.py b/tests/integration/test_background_task.py index 87aa1459b..00cf83ea0 100644 --- a/tests/integration/test_background_task.py +++ b/tests/integration/test_background_task.py @@ -189,7 +189,7 @@ def background_task( running AppHarness instance """ with AppHarness.create( - root=tmp_path_factory.mktemp(f"background_task"), + root=tmp_path_factory.mktemp("background_task"), app_source=BackgroundTask, # type: ignore ) as harness: yield harness diff --git a/tests/integration/test_computed_vars.py b/tests/integration/test_computed_vars.py index 1f585cd8b..a39cb9e65 100644 --- a/tests/integration/test_computed_vars.py +++ b/tests/integration/test_computed_vars.py @@ -124,7 +124,7 @@ def computed_vars( running AppHarness instance """ with AppHarness.create( - root=tmp_path_factory.mktemp(f"computed_vars"), + root=tmp_path_factory.mktemp("computed_vars"), app_source=ComputedVars, # type: ignore ) as harness: yield harness diff --git a/tests/integration/test_dynamic_routes.py b/tests/integration/test_dynamic_routes.py index 31b7fa419..ffb4b1c3c 100644 --- a/tests/integration/test_dynamic_routes.py +++ b/tests/integration/test_dynamic_routes.py @@ -153,7 +153,7 @@ def dynamic_route( running AppHarness instance """ with app_harness_env.create( - root=tmp_path_factory.mktemp(f"dynamic_route"), + root=tmp_path_factory.mktemp("dynamic_route"), app_name=f"dynamicroute_{app_harness_env.__name__.lower()}", app_source=DynamicRoute, # type: ignore ) as harness: diff --git a/tests/integration/test_event_actions.py b/tests/integration/test_event_actions.py index 5d278835e..58e26bf09 100644 --- a/tests/integration/test_event_actions.py +++ b/tests/integration/test_event_actions.py @@ -171,7 +171,7 @@ def event_action(tmp_path_factory) -> Generator[AppHarness, None, None]: running AppHarness instance """ with AppHarness.create( - root=tmp_path_factory.mktemp(f"event_action"), + root=tmp_path_factory.mktemp("event_action"), app_source=TestEventAction, # type: ignore ) as harness: yield harness diff --git a/tests/integration/test_large_state.py b/tests/integration/test_large_state.py index 23a534a2b..a9a8ff2ec 100644 --- a/tests/integration/test_large_state.py +++ b/tests/integration/test_large_state.py @@ -58,7 +58,7 @@ def test_large_state(var_count: int, tmp_path_factory, benchmark): large_state_rendered = template.render(var_count=var_count) with AppHarness.create( - root=tmp_path_factory.mktemp(f"large_state"), + root=tmp_path_factory.mktemp("large_state"), app_source=large_state_rendered, app_name="large_state", ) as large_state: diff --git a/tests/integration/test_navigation.py b/tests/integration/test_navigation.py index 492ae4510..b4505ed1c 100644 --- a/tests/integration/test_navigation.py +++ b/tests/integration/test_navigation.py @@ -74,7 +74,7 @@ async def test_navigation_app(navigation_app: AppHarness): with poll_for_navigation(driver): internal_link.click() - assert urlsplit(driver.current_url).path == f"/internal/" + assert urlsplit(driver.current_url).path == "/internal/" with poll_for_navigation(driver): driver.back() diff --git a/tests/integration/test_server_side_event.py b/tests/integration/test_server_side_event.py index cacf6e1c5..7fd592d10 100644 --- a/tests/integration/test_server_side_event.py +++ b/tests/integration/test_server_side_event.py @@ -102,7 +102,6 @@ def server_side_event(tmp_path_factory) -> Generator[AppHarness, None, None]: def driver(server_side_event: AppHarness): """Get an instance of the browser open to the server_side_event app. - Args: server_side_event: harness for ServerSideEvent app diff --git a/tests/integration/test_state_inheritance.py b/tests/integration/test_state_inheritance.py index 86ab625e1..a9f55d9a9 100644 --- a/tests/integration/test_state_inheritance.py +++ b/tests/integration/test_state_inheritance.py @@ -216,7 +216,7 @@ def state_inheritance( running AppHarness instance """ with AppHarness.create( - root=tmp_path_factory.mktemp(f"state_inheritance"), + root=tmp_path_factory.mktemp("state_inheritance"), app_source=StateInheritance, # type: ignore ) as harness: yield harness diff --git a/tests/integration/test_upload.py b/tests/integration/test_upload.py index 7eaabf6a1..602905b3e 100644 --- a/tests/integration/test_upload.py +++ b/tests/integration/test_upload.py @@ -359,8 +359,8 @@ async def test_cancel_upload(tmp_path, upload_file: AppHarness, driver: WebDrive substate_token = f"{token}_{state_full_name}" upload_box = driver.find_elements(By.XPATH, "//input[@type='file']")[1] - upload_button = driver.find_element(By.ID, f"upload_button_secondary") - cancel_button = driver.find_element(By.ID, f"cancel_button_secondary") + upload_button = driver.find_element(By.ID, "upload_button_secondary") + cancel_button = driver.find_element(By.ID, "cancel_button_secondary") exp_name = "large.txt" target_file = tmp_path / exp_name diff --git a/tests/units/compiler/test_compiler.py b/tests/units/compiler/test_compiler.py index afacf43c5..22f5c8483 100644 --- a/tests/units/compiler/test_compiler.py +++ b/tests/units/compiler/test_compiler.py @@ -131,11 +131,11 @@ def test_compile_stylesheets(tmp_path, mocker): assert compiler.compile_root_stylesheet(stylesheets) == ( str(Path(".web") / "styles" / "styles.css"), - f"@import url('./tailwind.css'); \n" - f"@import url('https://fonts.googleapis.com/css?family=Sofia&effect=neon|outline|emboss|shadow-multiple'); \n" - f"@import url('https://cdn.jsdelivr.net/npm/bootstrap@3.3.7/dist/css/bootstrap.min.css'); \n" - f"@import url('../public/styles.css'); \n" - f"@import url('https://cdn.jsdelivr.net/npm/bootstrap@3.3.7/dist/css/bootstrap-theme.min.css'); \n", + "@import url('./tailwind.css'); \n" + "@import url('https://fonts.googleapis.com/css?family=Sofia&effect=neon|outline|emboss|shadow-multiple'); \n" + "@import url('https://cdn.jsdelivr.net/npm/bootstrap@3.3.7/dist/css/bootstrap.min.css'); \n" + "@import url('../public/styles.css'); \n" + "@import url('https://cdn.jsdelivr.net/npm/bootstrap@3.3.7/dist/css/bootstrap-theme.min.css'); \n", ) diff --git a/tests/units/components/core/test_debounce.py b/tests/units/components/core/test_debounce.py index 7856ee090..2fad9c925 100644 --- a/tests/units/components/core/test_debounce.py +++ b/tests/units/components/core/test_debounce.py @@ -41,7 +41,6 @@ class S(BaseState): def on_change(self, v: str): """Dummy on_change handler. - Args: v: The changed value. """ diff --git a/tests/units/components/test_component.py b/tests/units/components/test_component.py index f2c0d50e9..111431ab3 100644 --- a/tests/units/components/test_component.py +++ b/tests/units/components/test_component.py @@ -1208,7 +1208,7 @@ TEST_VAR_DICT_OF_DICT = LiteralVar.create({"a": {"b": "test"}})._replace( merge_var_data=TEST_VAR._var_data ) FORMATTED_TEST_VAR_DICT_OF_DICT = LiteralVar.create( - {"a": {"b": f"footestbar"}} + {"a": {"b": "footestbar"}} )._replace(merge_var_data=TEST_VAR._var_data) TEST_VAR_LIST_OF_LIST = LiteralVar.create([["test"]])._replace( diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 1e34a67c3..5d3aee6c7 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -787,11 +787,11 @@ async def test_upload_file(tmp_path, state, delta, token: str, mocker): } file1 = UploadFile( - filename=f"image1.jpg", + filename="image1.jpg", file=bio, ) file2 = UploadFile( - filename=f"image2.jpg", + filename="image2.jpg", file=bio, ) upload_fn = upload(app) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 4cfa68254..7ffdcd79d 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -1559,7 +1559,7 @@ def test_error_on_state_method_shadow(): assert ( err.value.args[0] - == f"The event handler name `reset` shadows a builtin State method; use a different name instead" + == "The event handler name `reset` shadows a builtin State method; use a different name instead" ) diff --git a/tests/units/test_var.py b/tests/units/test_var.py index a8b4b759d..2f3f81e2c 100644 --- a/tests/units/test_var.py +++ b/tests/units/test_var.py @@ -1318,7 +1318,7 @@ def test_unsupported_types_for_reverse(var): """ with pytest.raises(TypeError) as err: var.reverse() - assert err.value.args[0] == f"Cannot reverse non-list var." + assert err.value.args[0] == "Cannot reverse non-list var." @pytest.mark.parametrize( diff --git a/tests/units/utils/test_serializers.py b/tests/units/utils/test_serializers.py index 8050470c6..355f40d3f 100644 --- a/tests/units/utils/test_serializers.py +++ b/tests/units/utils/test_serializers.py @@ -20,7 +20,6 @@ from reflex.vars.base import LiteralVar def test_has_serializer(type_: Type, expected: bool): """Test that has_serializer returns the correct value. - Args: type_: The type to check. expected: The expected result. @@ -41,7 +40,6 @@ def test_has_serializer(type_: Type, expected: bool): def test_get_serializer(type_: Type, expected: serializers.Serializer): """Test that get_serializer returns the correct value. - Args: type_: The type to check. expected: The expected result. @@ -195,7 +193,6 @@ class BaseSubclass(Base): def test_serialize(value: Any, expected: str): """Test that serialize returns the correct value. - Args: value: The value to serialize. expected: The expected result. From 387efeb39d28290c076a3b95dee7efb50623a3a6 Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 7 Nov 2024 20:39:45 +0100 Subject: [PATCH 04/11] ruffing --- tests/units/test_state_tree.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/units/test_state_tree.py b/tests/units/test_state_tree.py index aa853af9b..ebdd877de 100644 --- a/tests/units/test_state_tree.py +++ b/tests/units/test_state_tree.py @@ -6,12 +6,7 @@ import pytest import pytest_asyncio import reflex as rx -from reflex.state import ( - BaseState, - StateManager, - StateManagerRedis, - _substate_key, -) +from reflex.state import BaseState, StateManager, StateManagerRedis, _substate_key class Root(BaseState): From 83412fa9f141d4b57f6016bfd4cdc9ca98bd5c5d Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 7 Nov 2024 21:08:35 +0100 Subject: [PATCH 05/11] prefix all redis keys --- reflex/state.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 81a054173..657827b88 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -3403,7 +3403,7 @@ class StateManagerRedis(StateManager): while not state_is_locked: # wait for the lock to be released while True: - if not await self.redis.exists(lock_key): + if not await self.redis.exists(prefix_redis_token(lock_key)): break # key was removed, try to get the lock again message = await pubsub.get_message( ignore_subscribe_messages=True, @@ -3444,7 +3444,7 @@ class StateManagerRedis(StateManager): finally: if state_is_locked: # only delete our lock - await self.redis.delete(lock_key) + await self.redis.delete(prefix_redis_token(lock_key)) async def close(self): """Explicitly close the redis connection and connection_pool. From e01e1ebcb76fce9aafcd9670bb5eae80d5b44224 Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 21 Nov 2024 02:31:27 +0100 Subject: [PATCH 06/11] increase timeout --- tests/integration/test_background_task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_background_task.py b/tests/integration/test_background_task.py index d7fe20824..e66197242 100644 --- a/tests/integration/test_background_task.py +++ b/tests/integration/test_background_task.py @@ -285,7 +285,7 @@ def test_background_task( increment_button.click() yield_increment_button.click() blocking_pause_button.click() - assert background_task._poll_for(lambda: counter.text == "620", timeout=40) + assert background_task._poll_for(lambda: counter.text == "620", timeout=60) # all tasks should have exited and cleaned up assert background_task._poll_for( lambda: not background_task.app_instance.background_tasks # type: ignore From 4b2cec3784c2762d6d351400350016529e8c470c Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 21 Nov 2024 02:47:25 +0100 Subject: [PATCH 07/11] lru_cache prefix_redis_token --- reflex/state.py | 1 + 1 file changed, 1 insertion(+) diff --git a/reflex/state.py b/reflex/state.py index 4e526ce6b..042c99732 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -3141,6 +3141,7 @@ def _default_lock_expiration() -> int: TOKEN_TYPE = TypeVar("TOKEN_TYPE", str, bytes) +@functools.lru_cache def prefix_redis_token(token: TOKEN_TYPE) -> TOKEN_TYPE: """Prefix the token with the redis prefix. From b28f4ac1c63b3261e0118cb16cb84a7f0174a67a Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 21 Nov 2024 02:51:10 +0100 Subject: [PATCH 08/11] improve performance --- reflex/state.py | 32 +++++++++++++++++++++++--------- tests/units/test_state.py | 12 +++++++----- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 042c99732..41fa3cd40 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -3142,7 +3142,7 @@ TOKEN_TYPE = TypeVar("TOKEN_TYPE", str, bytes) @functools.lru_cache -def prefix_redis_token(token: TOKEN_TYPE) -> TOKEN_TYPE: +def prefix_redis_token_str(token: str) -> str: """Prefix the token with the redis prefix. Args: @@ -3154,11 +3154,25 @@ def prefix_redis_token(token: TOKEN_TYPE) -> TOKEN_TYPE: prefix = EnvironmentVariables.REFLEX_REDIS_PREFIX.get() if not prefix: return token - if isinstance(token, bytes): - return prefix.encode() + token return f"{prefix}{token}" +@functools.lru_cache +def prefix_redis_token_bytes(token: bytes) -> bytes: + """Prefix the token with the redis prefix. + + Args: + token: The token to prefix. + + Returns: + The prefixed token. + """ + prefix = EnvironmentVariables.REFLEX_REDIS_PREFIX.get() + if not prefix: + return token + return prefix.encode() + token + + class StateManagerRedis(StateManager): """A state manager that stores states in redis.""" @@ -3296,7 +3310,7 @@ class StateManagerRedis(StateManager): state = None # Fetch the serialized substate from redis. - redis_state = await self.redis.get(prefix_redis_token(token)) + redis_state = await self.redis.get(prefix_redis_token_str(token)) if redis_state is not None: # Deserialize the substate. @@ -3351,7 +3365,7 @@ class StateManagerRedis(StateManager): # Check that we're holding the lock. if ( lock_id is not None - and await self.redis.get(prefix_redis_token(self._lock_key(token))) + and await self.redis.get(prefix_redis_token_str(self._lock_key(token))) != lock_id ): raise LockExpiredError( @@ -3383,7 +3397,7 @@ class StateManagerRedis(StateManager): pickle_state = state._serialize() if pickle_state: await self.redis.set( - prefix_redis_token(_substate_key(client_token, state)), + prefix_redis_token_str(_substate_key(client_token, state)), pickle_state, ex=self.token_expiration, ) @@ -3433,7 +3447,7 @@ class StateManagerRedis(StateManager): True if the lock was obtained. """ return await self.redis.set( - prefix_redis_token(lock_key), + prefix_redis_token_bytes(lock_key), lock_id, px=self.lock_expiration, nx=True, # only set if it doesn't exist @@ -3468,7 +3482,7 @@ class StateManagerRedis(StateManager): while not state_is_locked: # wait for the lock to be released while True: - if not await self.redis.exists(prefix_redis_token(lock_key)): + if not await self.redis.exists(prefix_redis_token_bytes(lock_key)): break # key was removed, try to get the lock again message = await pubsub.get_message( ignore_subscribe_messages=True, @@ -3509,7 +3523,7 @@ class StateManagerRedis(StateManager): finally: if state_is_locked: # only delete our lock - await self.redis.delete(prefix_redis_token(lock_key)) + await self.redis.delete(prefix_redis_token_str(lock_key)) async def close(self): """Explicitly close the redis connection and connection_pool. diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 31f5493db..e1f96f4ce 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -40,7 +40,7 @@ from reflex.state import ( StateProxy, StateUpdate, _substate_key, - prefix_redis_token, + prefix_redis_token_str, ) from reflex.testing import chdir from reflex.utils import format, prerequisites, types @@ -1672,7 +1672,9 @@ async def test_state_manager_modify_state( """ async with state_manager.modify_state(substate_token) as state: if isinstance(state_manager, StateManagerRedis): - assert await state_manager.redis.get(prefix_redis_token(f"{token}_lock")) + assert await state_manager.redis.get( + prefix_redis_token_str(f"{token}_lock") + ) elif isinstance(state_manager, (StateManagerMemory, StateManagerDisk)): assert token in state_manager._states_locks assert state_manager._states_locks[token].locked() @@ -1683,7 +1685,7 @@ async def test_state_manager_modify_state( # lock should be dropped after exiting the context if isinstance(state_manager, StateManagerRedis): assert ( - await state_manager.redis.get(prefix_redis_token(f"{token}_lock")) + await state_manager.redis.get(prefix_redis_token_str(f"{token}_lock")) ) is None elif isinstance(state_manager, (StateManagerMemory, StateManagerDisk)): assert not state_manager._states_locks[token].locked() @@ -1727,7 +1729,7 @@ async def test_state_manager_contend( if isinstance(state_manager, StateManagerRedis): assert ( - await state_manager.redis.get(prefix_redis_token(f"{token}_lock")) + await state_manager.redis.get(prefix_redis_token_str(f"{token}_lock")) ) is None elif isinstance(state_manager, (StateManagerMemory, StateManagerDisk)): assert token in state_manager._states_locks @@ -1859,7 +1861,7 @@ async def test_state_manager_redis_prefix( async with state_manager_redis.modify_state(substate_token_redis) as state: state.num1 = 42 - prefixed_token = prefix_redis_token(substate_token_redis) + prefixed_token = prefix_redis_token_str(substate_token_redis) assert prefixed_token == f"{redis_prefix}{substate_token_redis}" assert await state_manager_redis.redis.get(prefixed_token) From b26dcfa798e25d05aaaf85287b8af279a57f378e Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 21 Nov 2024 02:56:46 +0100 Subject: [PATCH 09/11] fix wrong key type --- reflex/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reflex/state.py b/reflex/state.py index 41fa3cd40..87046db10 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -3365,7 +3365,7 @@ class StateManagerRedis(StateManager): # Check that we're holding the lock. if ( lock_id is not None - and await self.redis.get(prefix_redis_token_str(self._lock_key(token))) + and await self.redis.get(prefix_redis_token_bytes(self._lock_key(token))) != lock_id ): raise LockExpiredError( From 7f0e0542d2ec95649e26f10644d1451ca500573e Mon Sep 17 00:00:00 2001 From: Benedikt Bartscher Date: Thu, 21 Nov 2024 21:38:21 +0100 Subject: [PATCH 10/11] Revert "increase timeout" This reverts commit e01e1ebcb76fce9aafcd9670bb5eae80d5b44224. --- tests/integration/test_background_task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_background_task.py b/tests/integration/test_background_task.py index e66197242..d7fe20824 100644 --- a/tests/integration/test_background_task.py +++ b/tests/integration/test_background_task.py @@ -285,7 +285,7 @@ def test_background_task( increment_button.click() yield_increment_button.click() blocking_pause_button.click() - assert background_task._poll_for(lambda: counter.text == "620", timeout=60) + assert background_task._poll_for(lambda: counter.text == "620", timeout=40) # all tasks should have exited and cleaned up assert background_task._poll_for( lambda: not background_task.app_instance.background_tasks # type: ignore From 2eedf15322423c68e494abd4f9491c51e184b7fb Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Thu, 21 Nov 2024 17:14:09 -0800 Subject: [PATCH 11/11] Simplify redis prefix for lock key --- reflex/state.py | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 87046db10..a1a478597 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -3157,22 +3157,6 @@ def prefix_redis_token_str(token: str) -> str: return f"{prefix}{token}" -@functools.lru_cache -def prefix_redis_token_bytes(token: bytes) -> bytes: - """Prefix the token with the redis prefix. - - Args: - token: The token to prefix. - - Returns: - The prefixed token. - """ - prefix = EnvironmentVariables.REFLEX_REDIS_PREFIX.get() - if not prefix: - return token - return prefix.encode() + token - - class StateManagerRedis(StateManager): """A state manager that stores states in redis.""" @@ -3365,8 +3349,7 @@ class StateManagerRedis(StateManager): # Check that we're holding the lock. if ( lock_id is not None - and await self.redis.get(prefix_redis_token_bytes(self._lock_key(token))) - != lock_id + and await self.redis.get(self._lock_key(token)) != lock_id ): raise LockExpiredError( f"Lock expired for token {token} while processing. Consider increasing " @@ -3434,7 +3417,7 @@ class StateManagerRedis(StateManager): """ # All substates share the same lock domain, so ignore any substate path suffix. client_token = _split_substate_key(token)[0] - return f"{client_token}_lock".encode() + return prefix_redis_token_str(f"{client_token}_lock").encode() async def _try_get_lock(self, lock_key: bytes, lock_id: bytes) -> bool | None: """Try to get a redis lock for a token. @@ -3447,7 +3430,7 @@ class StateManagerRedis(StateManager): True if the lock was obtained. """ return await self.redis.set( - prefix_redis_token_bytes(lock_key), + lock_key, lock_id, px=self.lock_expiration, nx=True, # only set if it doesn't exist @@ -3482,7 +3465,7 @@ class StateManagerRedis(StateManager): while not state_is_locked: # wait for the lock to be released while True: - if not await self.redis.exists(prefix_redis_token_bytes(lock_key)): + if not await self.redis.exists(lock_key): break # key was removed, try to get the lock again message = await pubsub.get_message( ignore_subscribe_messages=True, @@ -3523,7 +3506,7 @@ class StateManagerRedis(StateManager): finally: if state_is_locked: # only delete our lock - await self.redis.delete(prefix_redis_token_str(lock_key)) + await self.redis.delete(lock_key) async def close(self): """Explicitly close the redis connection and connection_pool.