mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-27 21:50:36 +00:00
fix(config): honor $VAR substitution for request_admission integer fields (#5838)
* fix(config): honor $VAR substitution for request_admission integer fields AppConfig.resolve_env_variables replaces every `$VAR` reference with the raw environment string. RequestAdmissionConfig.requests_per_minute and max_queue_size are declared `strict=True` so a bool or float cannot slip in, but strict mode also refuses the string "60", so `requests_per_minute: $RPM` failed the whole config load with "Input should be a valid integer" even when RPM=60 — contradicting config.example.yaml's promise that environment variables work for every field. They were the only strict fields in the config schema; every other integer coerces in lax mode. Wrap both fields in a BeforeValidator that converts a decimal integer literal arriving as a string and leaves everything else untouched, so the strict check behind it still rejects bools, floats, "60.0" and non-numeric strings. Tests pin the accepted literals, the rejections that must survive, and the end-to-end path from a config.yaml `$VAR` reference through AppConfig.from_file. * docs(changelog): reference #5838 in the request_admission entries
This commit is contained in:
parent
3a86278047
commit
3448c3426e
@ -957,6 +957,13 @@ This release closes that milestone with **765 merged pull requests**.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **config:** `request_admission.requests_per_minute` and `max_queue_size` now
|
||||
accept `$VAR` environment references like every other field. Both are strict
|
||||
integers so a bool or float is still rejected, but `$VAR` substitution always
|
||||
produces a string, so `requests_per_minute: $RPM` failed the whole config load
|
||||
with "Input should be a valid integer" even when `RPM=60`. A decimal literal
|
||||
delivered as a string is now converted before the strict check; any other
|
||||
string is still rejected. ([#5838])
|
||||
- **scheduler:** Pausing a scheduled task no longer loses the pause when a
|
||||
dispatch is in flight on SQLite. `release_dispatch_lease` guards on the lease
|
||||
owner — which pausing clears — but read the row without taking SQLite's
|
||||
@ -4420,4 +4427,5 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#5734]: https://github.com/bytedance/deer-flow/pull/5734
|
||||
[#5776]: https://github.com/bytedance/deer-flow/pull/5776
|
||||
[#5777]: https://github.com/bytedance/deer-flow/pull/5777
|
||||
[#5838]: https://github.com/bytedance/deer-flow/pull/5838
|
||||
|
||||
|
||||
@ -727,6 +727,11 @@
|
||||
|
||||
### 修复
|
||||
|
||||
- **配置:** `request_admission.requests_per_minute` 与 `max_queue_size` 现在与其他字段一样
|
||||
接受 `$VAR` 环境变量引用。这两个字段是严格整数,布尔值与浮点数仍会被拒绝;但 `$VAR`
|
||||
替换得到的永远是字符串,因此即使 `RPM=60`,`requests_per_minute: $RPM` 也会让整个配置
|
||||
加载失败并报 "Input should be a valid integer"。现在以字符串形式到达的十进制整数字面量会在
|
||||
严格校验之前被转换;其他字符串仍会被拒绝。([#5838])
|
||||
- **调度器:** 在 SQLite 上,调度分发进行中暂停计划任务时不再丢失暂停状态。
|
||||
`release_dispatch_lease` 依据租约持有者做校验(暂停会清除该字段),但读取任务行时没有先获取
|
||||
SQLite 的写锁,因此过期的读取会通过校验,并把任务状态写回 `enabled` 且不改动 `next_run_at`,
|
||||
@ -3572,3 +3577,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
|
||||
[#5734]: https://github.com/bytedance/deer-flow/pull/5734
|
||||
[#5776]: https://github.com/bytedance/deer-flow/pull/5776
|
||||
[#5777]: https://github.com/bytedance/deer-flow/pull/5777
|
||||
[#5838]: https://github.com/bytedance/deer-flow/pull/5838
|
||||
|
||||
@ -63,6 +63,10 @@ request_admission:
|
||||
max_queue_size: 256
|
||||
```
|
||||
|
||||
Like every other field, these accept `$VAR` environment references; an integer
|
||||
field such as `requests_per_minute: $RPM` validates when the variable holds a
|
||||
decimal integer.
|
||||
|
||||
Calls wait in a bounded FIFO before dispatch. At 60 RPM, admissions are spaced
|
||||
at least one second apart, even after idle periods. The first call can proceed
|
||||
immediately. Async waiting is cancellable; a cancelled or expired waiter spends
|
||||
|
||||
@ -62,7 +62,10 @@ Config values starting with `$` are resolved as environment variables (e.g., `$O
|
||||
|
||||
`ModelConfig.request_admission` is optional and is not a provider parameter.
|
||||
Its positive RPM, finite wait deadline, queue bound and optional quota-group name
|
||||
configure process-local model pacing. Models sharing an explicit group must use
|
||||
configure process-local model pacing. `requests_per_minute` and `max_queue_size` are strict
|
||||
integers (bools and floats are rejected) behind a `BeforeValidator` that converts a
|
||||
decimal literal delivered as a string, because `$VAR` substitution always yields
|
||||
`str`; any other string still fails validation. Models sharing an explicit group must use
|
||||
identical policies. Restart after changing, disabling or regrouping an active
|
||||
policy; conflicting policies fail construction rather than silently resetting
|
||||
an active budget. This nested model option is enforced by its limiter registry,
|
||||
|
||||
@ -1,16 +1,37 @@
|
||||
from typing import Literal
|
||||
import re
|
||||
from typing import Annotated, Literal
|
||||
|
||||
from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
|
||||
from pydantic import BaseModel, BeforeValidator, ConfigDict, Field, field_validator, model_validator
|
||||
|
||||
_INTEGER_LITERAL_PATTERN = re.compile(r"[0-9]+")
|
||||
|
||||
|
||||
def _accept_integer_literal_string(value: object) -> object:
|
||||
"""Turn an integer literal that arrived as a string into an ``int``.
|
||||
|
||||
``AppConfig.resolve_env_variables`` substitutes every ``$VAR`` reference
|
||||
with the raw environment string, so a strict integer field would reject
|
||||
``requests_per_minute: $RPM`` even when ``RPM=60``. Only a plain decimal
|
||||
literal is converted; everything else is returned untouched so the strict
|
||||
validation behind it still rejects bools, floats and non-integer strings.
|
||||
"""
|
||||
if isinstance(value, str) and _INTEGER_LITERAL_PATTERN.fullmatch(value.strip()):
|
||||
return int(value.strip())
|
||||
return value
|
||||
|
||||
|
||||
# A strict ``int`` (no bool/float coercion) that still honors ``$VAR`` substitution.
|
||||
StrictIntFromEnv = Annotated[int, BeforeValidator(_accept_integer_literal_string)]
|
||||
|
||||
|
||||
class RequestAdmissionConfig(BaseModel):
|
||||
"""Optional per-process pacing of model requests, shared by quota group."""
|
||||
|
||||
model_config = ConfigDict(extra="forbid", frozen=True)
|
||||
requests_per_minute: int = Field(gt=0, strict=True)
|
||||
requests_per_minute: StrictIntFromEnv = Field(gt=0, strict=True)
|
||||
group: str | None = Field(default=None, min_length=1, max_length=100, pattern=r"^[A-Za-z0-9_.-]+$")
|
||||
max_wait_seconds: float = Field(default=300, gt=0, allow_inf_nan=False)
|
||||
max_queue_size: int = Field(default=256, gt=0, strict=True)
|
||||
max_queue_size: StrictIntFromEnv = Field(default=256, gt=0, strict=True)
|
||||
|
||||
|
||||
_EFFORT_TOKEN_PATTERN = r"^[A-Za-z0-9_.-]{1,32}$"
|
||||
|
||||
@ -5,6 +5,7 @@ import threading
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
from pydantic import ValidationError
|
||||
|
||||
from deerflow.config.model_config import RequestAdmissionConfig
|
||||
@ -97,6 +98,71 @@ def test_invalid_configuration(values):
|
||||
RequestAdmissionConfig(**values)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"values",
|
||||
[
|
||||
{"requests_per_minute": "60"},
|
||||
{"requests_per_minute": " 60 "},
|
||||
{"requests_per_minute": 1, "max_queue_size": "512"},
|
||||
],
|
||||
)
|
||||
def test_integer_literal_strings_are_accepted(values):
|
||||
"""``$VAR`` substitution always yields ``str``; an integer literal must still validate."""
|
||||
config = RequestAdmissionConfig(**values)
|
||||
assert config.requests_per_minute == int(str(values["requests_per_minute"]).strip())
|
||||
assert config.max_queue_size == int(str(values.get("max_queue_size", 256)).strip())
|
||||
assert type(config.requests_per_minute) is int
|
||||
assert type(config.max_queue_size) is int
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"values",
|
||||
[
|
||||
{"requests_per_minute": 60.0},
|
||||
{"requests_per_minute": "60.0"},
|
||||
{"requests_per_minute": "sixty"},
|
||||
{"requests_per_minute": ""},
|
||||
{"requests_per_minute": "0"},
|
||||
{"requests_per_minute": 1, "max_queue_size": True},
|
||||
{"requests_per_minute": 1, "max_queue_size": 256.0},
|
||||
{"requests_per_minute": 1, "max_queue_size": "-1"},
|
||||
],
|
||||
)
|
||||
def test_non_integer_inputs_remain_rejected(values):
|
||||
"""Accepting env-substituted integer strings must not reopen the strict check for bools, floats, or other strings."""
|
||||
with pytest.raises(ValidationError):
|
||||
RequestAdmissionConfig(**values)
|
||||
|
||||
|
||||
def test_env_substitution_reaches_request_admission_from_config_file(monkeypatch, tmp_path):
|
||||
"""``config.example.yaml`` promises ``$VAR`` for every field; the two strict integers must honor it end to end."""
|
||||
from deerflow.config.app_config import AppConfig
|
||||
|
||||
monkeypatch.setenv("DEER_FLOW_TEST_RPM", "60")
|
||||
monkeypatch.setenv("DEER_FLOW_TEST_QUEUE", "512")
|
||||
path = tmp_path / "config.yaml"
|
||||
path.write_text(
|
||||
yaml.safe_dump(
|
||||
{
|
||||
"sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"},
|
||||
"models": [
|
||||
{
|
||||
"name": "paced",
|
||||
"use": "langchain_openai:ChatOpenAI",
|
||||
"model": "gpt-test",
|
||||
"request_admission": {"requests_per_minute": "$DEER_FLOW_TEST_RPM", "max_queue_size": "$DEER_FLOW_TEST_QUEUE"},
|
||||
}
|
||||
],
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
policy = AppConfig.from_file(str(path)).get_model_config("paced").request_admission
|
||||
|
||||
assert policy == RequestAdmissionConfig(requests_per_minute=60, max_queue_size=512)
|
||||
|
||||
|
||||
def test_sync_timeout_removes_waiter(clock, monkeypatch):
|
||||
limiter = admission.RequestAdmission(RequestAdmissionConfig(requests_per_minute=1, max_wait_seconds=1))
|
||||
limiter.acquire()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user