diff --git a/shard_core/service/app_lifecycle.py b/shard_core/service/app_lifecycle.py index 79e690e..2a59c53 100644 --- a/shard_core/service/app_lifecycle.py +++ b/shard_core/service/app_lifecycle.py @@ -37,7 +37,7 @@ async def ensure_app_is_running(app: InstalledApp): last_access_dict[app.name] = time.time() # PAUSED wakes via unpause (ms to ~2s), everything else via the legacy # start path. Calling docker_unpause_app directly also dodges the - # global 5s throttle on docker_start_app, which would silently drop + # per-app 5s throttle on docker_start_app, which would silently drop # the wake. This dispatch stays active even with pause_enabled=false, # so already-paused apps still wake after a backout. if app.status == Status.PAUSED: diff --git a/shard_core/service/app_tools.py b/shard_core/service/app_tools.py index b7fdc8c..1d304ab 100644 --- a/shard_core/service/app_tools.py +++ b/shard_core/service/app_tools.py @@ -31,7 +31,7 @@ async def docker_create_app_containers(name: str): await subprocess(*_app_compose(name), "up", "--no-start") -@throttle(5) +@throttle(5, key=lambda name: name) async def docker_start_app(name: str): async with db_conn() as conn: app = await db_installed_apps.get_by_name(conn, name) diff --git a/shard_core/util/misc.py b/shard_core/util/misc.py index 5253758..5db1732 100644 --- a/shard_core/util/misc.py +++ b/shard_core/util/misc.py @@ -2,26 +2,39 @@ import time -def throttle(min_duration: float): +def throttle(min_duration: float, key=None): + """Drop calls that arrive within min_duration of the previous accepted call. + + key maps the call args to a bucket key so each bucket throttles + independently; key=None (default) uses one global window for the function. + The wrapper exposes reset() to clear all windows (test hook). + """ + def decorator_throttle(func): - last_call = None + last_call: dict = {} + + def is_allowed(*args, **kwargs): + k = key(*args, **kwargs) if key is not None else None + now = time.time() + prev = last_call.get(k) + if prev is None or prev + min_duration < now: + last_call[k] = now + return True + return False if inspect.iscoroutinefunction(func): async def wrapper_throttle(*args, **kwargs): - nonlocal last_call - if last_call is None or last_call + min_duration < time.time(): - last_call = time.time() + if is_allowed(*args, **kwargs): return await func(*args, **kwargs) else: def wrapper_throttle(*args, **kwargs): - nonlocal last_call - if last_call is None or last_call + min_duration < time.time(): - last_call = time.time() + if is_allowed(*args, **kwargs): return func(*args, **kwargs) + wrapper_throttle.reset = last_call.clear return wrapper_throttle return decorator_throttle diff --git a/tests/test_app_compose_pinning.py b/tests/test_app_compose_pinning.py index 8b2793b..2e5c462 100644 --- a/tests/test_app_compose_pinning.py +++ b/tests/test_app_compose_pinning.py @@ -95,11 +95,9 @@ def subprocess_mock(): @pytest.fixture(autouse=True) def reset_start_throttle(): - """docker_start_app's @throttle(5) is global across apps and tests — a start - triggered by an earlier test would silently drop our call.""" - wrapper = app_tools.docker_start_app - cell = wrapper.__closure__[wrapper.__code__.co_freevars.index("last_call")] - cell.cell_contents = None + """docker_start_app's @throttle(5) is per-app but persists across tests — a + start of the same app in an earlier test would silently drop our call.""" + app_tools.docker_start_app.reset() async def _insert_app(name: str, status: Status): @@ -148,3 +146,43 @@ async def test_app_operation_with_compose_file_pins_the_app_project( assert "-f" in command and command[command.index("-f") + 1] == str( app_dir / "docker-compose.yml" ) + + +def _started_apps(subprocess_mock) -> list[str]: + return [ + c.args[c.args.index("-p") + 1] + for c in subprocess_mock.call_args_list + if c.args[-2:] == ("up", "-d") + ] + + +async def test_docker_start_app_starts_each_app_despite_throttle( + db, tmp_path, subprocess_mock +): + _app_dir(tmp_path, "appa") + _app_dir(tmp_path, "appb") + await _insert_app("appa", Status.STOPPED) + await _insert_app("appb", Status.STOPPED) + + with settings_override({"path_root": str(tmp_path)}): + await app_tools.docker_start_app("appa") + await app_tools.docker_start_app("appb") + + assert _started_apps(subprocess_mock) == ["appa", "appb"] + + +async def test_docker_start_app_throttles_repeated_start_of_same_app( + db, tmp_path, subprocess_mock +): + _app_dir(tmp_path, "appa") + await _insert_app("appa", Status.STOPPED) + + with settings_override({"path_root": str(tmp_path)}): + await app_tools.docker_start_app("appa") + # force back to a startable status so only the throttle, not status + # gating, can drop the second start + async with db_conn() as conn: + await db_installed_apps.update_status(conn, "appa", Status.STOPPED) + await app_tools.docker_start_app("appa") + + assert _started_apps(subprocess_mock) == ["appa"] diff --git a/tests/test_throttle.py b/tests/test_throttle.py index df1e9e8..127f970 100644 --- a/tests/test_throttle.py +++ b/tests/test_throttle.py @@ -28,3 +28,25 @@ def call_me(): assert call_me() == "called" time.sleep(0.2) assert call_me() == "called" + + +def test_throttle_per_key_windows_are_independent(): + @throttle(0.1, key=lambda name: name) + def call_me(name): + return f"called {name}" + + assert call_me("a") == "called a" + assert call_me("b") == "called b" + assert call_me("a") is None + assert call_me("b") is None + + +def test_throttle_reset_clears_all_windows(): + @throttle(0.1, key=lambda name: name) + def call_me(name): + return f"called {name}" + + assert call_me("a") == "called a" + assert call_me("a") is None + call_me.reset() + assert call_me("a") == "called a"