Skip to content

Commit d830644

Browse files
committed
LCORE-4606: reject quota limiter periods SQLite cannot use
With the SQLite quota storage the scheduler passes the limiter period to datetime() as a modifier. For a period SQLite cannot parse, such as "1 week" or "1 day 2 hours", datetime() returns NULL, the renewal condition is never true and the quota is never renewed, with no error. QuotaHandlersConfiguration now checks every limiter period when sqlite is configured and fails the configuration load with an error that names the limiter and the period. SQLite itself is asked: a period is accepted when datetime() moves a fixed timestamp forward, so zero and negative periods are rejected as well. Nothing is checked without sqlite. Unit tests cover accepted and rejected periods with sqlite, "1 week" with postgres only and with no storage, and the helper. Five existing test configurations paired sqlite with filler text as the period; they now use periods SQLite accepts.
1 parent d8eb7cf commit d830644

4 files changed

Lines changed: 173 additions & 10 deletions

File tree

‎src/models/config.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2600,6 +2600,37 @@ class QuotaHandlersConfiguration(ConfigurationBase):
26002600
description="Enables storing information about token usage history",
26012601
)
26022602

2603+
@model_validator(mode="after")
2604+
def check_limiter_periods(self) -> Self:
2605+
"""
2606+
Ensure that the SQLite storage can use the period of every limiter.
2607+
2608+
The quota scheduler passes the period to the SQLite function datetime()
2609+
as a modifier. For a period SQLite can not parse datetime() returns
2610+
NULL and the quota is never renewed, without any error. Such a period
2611+
is rejected, and so is a period that does not move the time forward.
2612+
Nothing is checked when SQLite is not configured: PostgreSQL accepts a
2613+
wider syntax.
2614+
2615+
Returns:
2616+
Self: The validated configuration instance.
2617+
2618+
Raises:
2619+
ValueError: If SQLite is configured and it can not use the period
2620+
of a limiter.
2621+
"""
2622+
if self.sqlite is None:
2623+
return self
2624+
for limiter in self.limiters:
2625+
if not checks.is_valid_sqlite_period(limiter.period):
2626+
raise ValueError(
2627+
f"Quota limiter '{limiter.name}': period '{limiter.period}' "
2628+
"can not be used with the SQLite storage. Use one positive "
2629+
"number followed by seconds, minutes, hours, days, months "
2630+
"or years, for example '7 days'."
2631+
)
2632+
return self
2633+
26032634

26042635
class RerankerConfiguration(ConfigurationBase):
26052636
"""Reranker configuration for RAG chunk reranking."""

‎src/utils/checks.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33
import importlib
44
import importlib.util
55
import os
6+
import sqlite3
7+
from contextlib import closing
68
from types import ModuleType
79
from typing import Optional
810

911
from pydantic import FilePath
1012

13+
# fixed point in time the quota limiter periods are tried on
14+
SQLITE_PERIOD_REFERENCE = "2000-01-01 00:00:00"
15+
1116

1217
class InvalidConfigurationError(Exception):
1318
"""Lightspeed configuration is invalid."""
@@ -153,3 +158,32 @@ def is_valid_profile(profile_module: ModuleType) -> bool:
153158
return False
154159

155160
return isinstance(profile_config.get("system_prompts"), dict)
161+
162+
163+
def is_valid_sqlite_period(period: str) -> bool:
164+
"""
165+
Check whether SQLite can use the period to move a timestamp forward.
166+
167+
The quota scheduler passes the period of a quota limiter to the SQLite
168+
function datetime() as a modifier. SQLite itself is asked what it makes of
169+
the period, so the check can not drift from what SQLite accepts: datetime()
170+
returns NULL for a modifier it can not parse, and a zero or negative period
171+
does not give a later time.
172+
173+
Parameters:
174+
----------
175+
period (str): Period as specified in the quota limiter configuration.
176+
177+
Returns:
178+
-------
179+
bool: True if datetime() applied to a fixed timestamp with the period
180+
as the modifier returns a later time, False otherwise.
181+
"""
182+
# julianday() makes it a comparison of times: as text, the result of the
183+
# modifier 'subsec' would be greater than the timestamp it does not move
184+
with closing(sqlite3.connect(":memory:")) as connection:
185+
(is_later,) = connection.execute(
186+
"SELECT julianday(datetime(?, ?)) > julianday(?)",
187+
(SQLITE_PERIOD_REFERENCE, period, SQLITE_PERIOD_REFERENCE),
188+
).fetchone()
189+
return bool(is_later)

‎tests/unit/models/config/test_quota_handlers_config.py‎

Lines changed: 89 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ def test_quota_handlers_configuration_improper_values(subtests: SubTests) -> Non
265265
"name": "Christina Hall",
266266
"initial_quota": 723,
267267
"quota_increase": 112,
268-
"period": "Media pretty recently push gas.",
268+
"period": "1 day",
269269
}
270270
],
271271
"scheduler": {
@@ -284,14 +284,14 @@ def test_quota_handlers_configuration_improper_values(subtests: SubTests) -> Non
284284
"name": "Xavier Anthony",
285285
"initial_quota": 536,
286286
"quota_increase": 80,
287-
"period": "Wrong class strategy.",
287+
"period": "30 seconds",
288288
},
289289
{
290290
"type": "user_limiter",
291291
"name": "Caroline Weaver",
292292
"initial_quota": 31,
293293
"quota_increase": 57,
294-
"period": "Lead boy least base.",
294+
"period": "2 hours",
295295
},
296296
],
297297
"scheduler": {
@@ -388,7 +388,7 @@ def test_quota_handlers_configuration_improper_values(subtests: SubTests) -> Non
388388
"name": "Bill Boyd",
389389
"initial_quota": 582,
390390
"quota_increase": 84,
391-
"period": "Side no born set. Different weight speak why daugh",
391+
"period": "7 days",
392392
}
393393
],
394394
"scheduler": {
@@ -416,21 +416,21 @@ def test_quota_handlers_configuration_improper_values(subtests: SubTests) -> Non
416416
"name": "William Armstrong",
417417
"initial_quota": 305,
418418
"quota_increase": 648,
419-
"period": "Relate couple song way wind rule model.",
419+
"period": "5 minutes",
420420
},
421421
{
422422
"type": "user_limiter",
423423
"name": "Terry Mitchell",
424424
"initial_quota": 206,
425425
"quota_increase": 316,
426-
"period": "Onto within arrive type group. Black none human re",
426+
"period": "1 month",
427427
},
428428
{
429429
"type": "user_limiter",
430430
"name": "Deborah Vazquez",
431431
"initial_quota": 24,
432432
"quota_increase": 24,
433-
"period": "Call close table.",
433+
"period": "1 year",
434434
},
435435
],
436436
"scheduler": {
@@ -458,21 +458,21 @@ def test_quota_handlers_configuration_improper_values(subtests: SubTests) -> Non
458458
"name": "James Burgess",
459459
"initial_quota": 698,
460460
"quota_increase": 945,
461-
"period": "Employee stage activity total.",
461+
"period": "3 seconds",
462462
},
463463
{
464464
"type": "user_limiter",
465465
"name": "Brenda Richard",
466466
"initial_quota": 583,
467467
"quota_increase": 582,
468-
"period": "Among or only nearly.",
468+
"period": "12 hours",
469469
},
470470
{
471471
"type": "user_limiter",
472472
"name": "Carol Colon",
473473
"initial_quota": 26,
474474
"quota_increase": 527,
475-
"period": "Machine pull finally stock without us him.",
475+
"period": "30 days",
476476
},
477477
],
478478
"scheduler": {
@@ -748,3 +748,82 @@ def test_quota_handlers_from_dict_negative_cases(config_dict: dict[str, Any]) ->
748748
# try to initialize the app config and load configuration from a Python
749749
# dictionary
750750
QuotaHandlersConfiguration(**config_dict)
751+
752+
753+
sqlite_storage = {"sqlite": {"db_path": ":memory:"}}
754+
755+
postgres_storage = {"postgres": {"db": "quota", "user": "quota", "password": "secret"}}
756+
757+
no_storage: dict[str, Any] = {}
758+
759+
760+
def configuration_with_period(storage: dict[str, Any], period: str) -> dict[str, Any]:
761+
"""Return quota handlers configuration whose second limiter has the given period."""
762+
return {
763+
**storage,
764+
"limiters": [
765+
{
766+
"type": "cluster_limiter",
767+
"name": "cluster_daily_limits",
768+
"initial_quota": 1000,
769+
"quota_increase": 0,
770+
"period": "1 day",
771+
},
772+
{
773+
"type": "user_limiter",
774+
"name": "user_weekly_limits",
775+
"initial_quota": 100,
776+
"quota_increase": 0,
777+
"period": period,
778+
},
779+
],
780+
}
781+
782+
783+
periods_usable_by_sqlite = [
784+
"3 seconds",
785+
"30 minutes",
786+
"2 hours",
787+
"7 days",
788+
"1 month",
789+
"1 year",
790+
]
791+
792+
793+
@pytest.mark.parametrize("period", periods_usable_by_sqlite)
794+
def test_quota_handlers_sqlite_usable_period(period: str) -> None:
795+
"""Test that a period SQLite can use is accepted with the SQLite storage."""
796+
cfg = QuotaHandlersConfiguration(
797+
**configuration_with_period(sqlite_storage, period)
798+
)
799+
assert cfg.limiters[1].period == period
800+
801+
802+
periods_not_usable_by_sqlite = [
803+
"1 week",
804+
"1 day 2 hours",
805+
"0 days",
806+
"-1 day",
807+
"",
808+
"forever",
809+
]
810+
811+
812+
@pytest.mark.parametrize("period", periods_not_usable_by_sqlite)
813+
def test_quota_handlers_sqlite_unusable_period(period: str) -> None:
814+
"""Test that a period SQLite can not use is rejected with the SQLite storage."""
815+
with pytest.raises(
816+
ValidationError,
817+
match=f"Quota limiter 'user_weekly_limits': period '{period}' can not be "
818+
"used with the SQLite storage.*for example '7 days'",
819+
):
820+
QuotaHandlersConfiguration(**configuration_with_period(sqlite_storage, period))
821+
822+
823+
@pytest.mark.parametrize("storage", [postgres_storage, no_storage])
824+
def test_quota_handlers_period_not_checked_without_sqlite(
825+
storage: dict[str, Any],
826+
) -> None:
827+
"""Test that the period is not checked when the SQLite storage is not configured."""
828+
cfg = QuotaHandlersConfiguration(**configuration_with_period(storage, "1 week"))
829+
assert cfg.limiters[1].period == "1 week"

‎tests/unit/utils/test_checks.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,3 +236,22 @@ def test_no_system_prompts_profile() -> None:
236236
result = checks.is_valid_profile(fetched_module)
237237

238238
assert result is False
239+
240+
241+
@pytest.mark.parametrize(
242+
("period", "expected"),
243+
[
244+
("7 days", True),
245+
("30 seconds", True),
246+
("1 month", True),
247+
("1 week", False),
248+
("1 day 2 hours", False),
249+
("0 days", False),
250+
("-1 day", False),
251+
("subsec", False),
252+
("", False),
253+
],
254+
)
255+
def test_is_valid_sqlite_period(period: str, expected: bool) -> None:
256+
"""Test which periods SQLite can use to move a timestamp forward."""
257+
assert checks.is_valid_sqlite_period(period) is expected

0 commit comments

Comments
 (0)