config: retire ServerArgs.derive; per-runner values are constructor arguments (#33887)
This commit is contained in:
+3
-2
@@ -46,9 +46,10 @@ class _ChunkKVMLARunner(MockMLAModelRunner):
|
||||
def __init__(self, **kwargs):
|
||||
super().__init__(**kwargs)
|
||||
from sglang.srt.runtime_context import get_context
|
||||
from sglang.test.test_utils import server_args_variant
|
||||
|
||||
self.server_args = self.server_args.derive(
|
||||
"attention-unittest",
|
||||
self.server_args = server_args_variant(
|
||||
self.server_args,
|
||||
disable_chunked_prefix_cache=False,
|
||||
flashinfer_mla_disable_ragged=False,
|
||||
)
|
||||
|
||||
@@ -261,9 +261,9 @@ class TestServerArgsScopedOverride(_IsolatedServerArgs):
|
||||
# unnamed fields keep their dataclass defaults
|
||||
self.assertEqual(published.tp_size, 1)
|
||||
|
||||
def test_fields_carry_provenance(self):
|
||||
published = get_context().override_server_args(tp_size=4).install()
|
||||
self.assertIn(("test-override", {"tp_size": 4}), published._runtime_mutations)
|
||||
def test_unknown_fields_are_rejected(self):
|
||||
with self.assertRaises(ValueError):
|
||||
get_context().override_server_args(not_a_config_field=1).install()
|
||||
|
||||
def test_restore_reinstates_previous_publish(self):
|
||||
previous = object()
|
||||
@@ -299,12 +299,10 @@ class TestServerArgsScopedOverride(_IsolatedServerArgs):
|
||||
|
||||
def test_installed_config_arms_the_strict_guard(self):
|
||||
# The published dummy must behave like a resolved config: bare writes
|
||||
# raise, and a differing value lives on a derived variant.
|
||||
# raise.
|
||||
published = get_context().override_server_args(tp_size=2).install()
|
||||
with self.assertRaises(AttributeError):
|
||||
published.tp_size = 4
|
||||
variant = published.derive("test", tp_size=4)
|
||||
self.assertEqual(variant.tp_size, 4)
|
||||
self.assertEqual(published.tp_size, 2)
|
||||
|
||||
def test_restore_resets_the_capture_seed(self):
|
||||
|
||||
@@ -118,9 +118,7 @@ class TestContextOverride(CustomTestCase):
|
||||
rc.get_context().override(
|
||||
"ModelRunner.configure_kv_cache_dtype", kv_cache_dtype="fp8_e4m3"
|
||||
)
|
||||
draft = ServerArgs(model_path="dummy").derive(
|
||||
"draft-build", kv_cache_dtype="bf16"
|
||||
)
|
||||
draft = ServerArgs(model_path="dummy", kv_cache_dtype="bf16")
|
||||
with rc.get_context().preserve_config():
|
||||
rc.get_context().set_server_args(draft)
|
||||
# Inside the scope the draft's bags are live...
|
||||
|
||||
@@ -1,79 +0,0 @@
|
||||
"""``ServerArgs.derive`` is the only way one config becomes another.
|
||||
|
||||
After resolution the instance is the process's read-only startup record and the
|
||||
object the config bags were projected from, so it cannot be mutated: a change to
|
||||
resolved config goes to the bags (``get_context().override``), and a config that
|
||||
differs for one runner or worker — a draft's context length, an encode worker's
|
||||
device — is a second object.
|
||||
"""
|
||||
|
||||
from sglang.test.ci.ci_register import register_cpu_ci
|
||||
|
||||
register_cpu_ci(est_time=5, suite="base-a-test-cpu")
|
||||
|
||||
import unittest
|
||||
|
||||
from sglang.srt.runtime_context import get_context, reset_context
|
||||
from sglang.srt.server_args import ServerArgs
|
||||
from sglang.test.test_utils import CustomTestCase
|
||||
|
||||
|
||||
class TestServerArgsDerive(CustomTestCase):
|
||||
def tearDown(self):
|
||||
reset_context()
|
||||
|
||||
def _resolved(self) -> ServerArgs:
|
||||
"""A config in the post-resolution state, without a real model."""
|
||||
server_args = ServerArgs(model_path="dummy")
|
||||
object.__setattr__(server_args, "_declarations_materialized", True)
|
||||
return server_args
|
||||
|
||||
def test_the_receiver_is_untouched(self):
|
||||
server_args = self._resolved()
|
||||
variant = server_args.derive("draft_worker.copy", context_length=1024)
|
||||
|
||||
self.assertEqual(variant.context_length, 1024)
|
||||
self.assertIsNot(variant, server_args)
|
||||
self.assertIsNone(server_args.context_length)
|
||||
|
||||
def test_a_published_config_rejects_assignment_but_still_derives(self):
|
||||
override = get_context().override_server_args(tp_size=2)
|
||||
published = override.install()
|
||||
self.addCleanup(override.restore)
|
||||
|
||||
with self.assertRaises(AttributeError):
|
||||
published.tp_size = 4
|
||||
|
||||
self.assertEqual(published.derive("worker", tp_size=4).tp_size, 4)
|
||||
self.assertEqual(published.tp_size, 2)
|
||||
|
||||
def test_the_variant_is_not_published_by_deriving(self):
|
||||
override = get_context().override_server_args(tp_size=2)
|
||||
published = override.install()
|
||||
self.addCleanup(override.restore)
|
||||
|
||||
published.derive("worker", tp_size=4)
|
||||
|
||||
self.assertIs(get_context().server_args, published)
|
||||
|
||||
def test_the_variant_is_frozen_too(self):
|
||||
variant = self._resolved().derive("worker", context_length=1024)
|
||||
with self.assertRaises(AttributeError):
|
||||
variant.context_length = 2048
|
||||
|
||||
def test_provenance_is_recorded(self):
|
||||
variant = self._resolved().derive("draft_worker.copy", watchdog_timeout=1.0)
|
||||
self.assertIn(
|
||||
("draft_worker.copy", {"watchdog_timeout": 1.0}),
|
||||
getattr(variant, "_runtime_mutations", []),
|
||||
)
|
||||
|
||||
def test_a_resolvable_field_joins_the_declaration_stash(self):
|
||||
"""So a re-resolution of the variant keeps the derived value."""
|
||||
variant = self._resolved().derive("draft_worker.build", kv_cache_dtype="bf16")
|
||||
stash = getattr(variant, "_resolved_overrides", [])
|
||||
self.assertIn(("draft_worker.build", {"kv_cache_dtype": "bf16"}), stash)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -10,9 +10,9 @@ baseline to lock in the progress.
|
||||
|
||||
There is no post-resolution mutation entry point on the instance any more:
|
||||
resolved config changes go to the context bags via
|
||||
``get_context().override(source, **fields)``, and a config that differs for one
|
||||
runner or worker is a separate object built with ``ServerArgs.derive(source,
|
||||
**fields)``. The baseline is therefore zero. ``ServerArgs.__setattr__`` raises
|
||||
``get_context().override(source, **fields)``, and a value that differs for one
|
||||
runner or worker travels as a constructor argument to it. The baseline is
|
||||
therefore zero. ``ServerArgs.__setattr__`` raises
|
||||
on a bare assignment after resolution; this ratchet catches the sites the tests
|
||||
never execute.
|
||||
"""
|
||||
@@ -70,8 +70,8 @@ class TestServerArgsMutationRatchet(CustomTestCase):
|
||||
f"{count} > baseline {_BASELINE}. Configuration is resolved in "
|
||||
"ServerArgs.__post_init__; declare through the pipeline "
|
||||
"(passes / declare_load_time_override), change resolved config "
|
||||
"with get_context().override(source, ...), or build a variant "
|
||||
"with server_args.derive(source, ...) — do not assign fields."
|
||||
"with get_context().override(source, ...), or hand the value "
|
||||
"to its runner as a constructor argument — do not assign fields."
|
||||
)
|
||||
if count < _BASELINE:
|
||||
self.fail(
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
"""``ServerArgs`` has no in-place mutation entry, and nothing calls one.
|
||||
|
||||
``ServerArgs.override(source, **fields)`` used to mutate a resolved instance;
|
||||
after resolution the fields are the record the config bags were projected from,
|
||||
so such a write desyncs every namespace reader. The method is gone and the two
|
||||
sanctioned replacements are ``get_context().override`` (post-publish, writes the
|
||||
bags) and ``ServerArgs.derive`` (a variant for another runner / process). Late
|
||||
launcher-stage resolution writes in place through
|
||||
``arg_groups.overrides.declare_late_resolution``, which refuses the published
|
||||
instance.
|
||||
``ServerArgs.override(source, **fields)`` used to mutate a resolved instance,
|
||||
and ``ServerArgs.derive(source, **fields)`` used to copy-and-edit one; after
|
||||
resolution the fields are the record the config bags were projected from, so a
|
||||
write desyncs every namespace reader, and a copy invites publishing stale
|
||||
variants. Both are gone: post-publish changes go to the bags
|
||||
(``get_context().override``), a value one runner or worker owns travels as a
|
||||
constructor argument, and late launcher-stage resolution writes in place
|
||||
through ``arg_groups.overrides.declare_late_resolution``, which refuses the
|
||||
published instance.
|
||||
|
||||
The textual half of this guard matters because the resolution pipeline's own file
|
||||
is exempt from the mutation ratchet: a ``self.override(...)`` there — exactly
|
||||
@@ -41,11 +42,19 @@ _EXCLUDED = ("multimodal_gen",)
|
||||
|
||||
|
||||
class TestNoServerArgsMutationEntry(CustomTestCase):
|
||||
def test_the_method_is_gone(self):
|
||||
def test_the_methods_are_gone(self):
|
||||
self.assertFalse(
|
||||
hasattr(ServerArgs, "override"),
|
||||
"ServerArgs.override is back; post-publish changes belong on the bags "
|
||||
"(get_context().override) and per-runner values on a derive() variant.",
|
||||
"(get_context().override); a value one runner owns travels as a "
|
||||
"constructor argument.",
|
||||
)
|
||||
self.assertFalse(
|
||||
hasattr(ServerArgs, "derive"),
|
||||
"ServerArgs.derive is back; a value one runner or worker owns travels "
|
||||
"as a constructor argument (draft_attention_backend, MMEncoder "
|
||||
"gpu_id), and test doubles copy via "
|
||||
"sglang.test.test_utils.server_args_variant.",
|
||||
)
|
||||
|
||||
def test_nothing_calls_an_instance_override(self):
|
||||
@@ -63,11 +72,31 @@ class TestNoServerArgsMutationEntry(CustomTestCase):
|
||||
self.fail(
|
||||
"in-place ServerArgs mutation call-sites:\n"
|
||||
+ "\n".join(offenders)
|
||||
+ "\n\nUse get_context().override(source, ...) for resolved config, "
|
||||
"server_args.derive(source, ...) for a per-runner variant, or "
|
||||
"declare_late_resolution(...) for pre-publish launcher resolution."
|
||||
+ "\n\nUse get_context().override(source, ...) for resolved config "
|
||||
"or declare_late_resolution(...) for pre-publish launcher "
|
||||
"resolution."
|
||||
)
|
||||
|
||||
def test_nothing_derives(self):
|
||||
"""A config is never copied-and-edited in the package: a value one
|
||||
runner consumes travels as a constructor argument, and test doubles
|
||||
copy via ``server_args_variant`` (test_utils)."""
|
||||
derive_pattern = re.compile(r"\.derive\(")
|
||||
offenders = []
|
||||
for path in sorted(_SGLANG_ROOT.rglob("*.py")):
|
||||
rel = path.relative_to(_SGLANG_ROOT).as_posix()
|
||||
if rel.startswith(_EXCLUDED):
|
||||
continue
|
||||
source = path.read_text()
|
||||
for match in derive_pattern.finditer(source):
|
||||
line = source.count("\n", 0, match.start()) + 1
|
||||
offenders.append(f"{rel}:{line}")
|
||||
self.assertFalse(
|
||||
offenders,
|
||||
".derive( call-sites in the package (the method no longer exists):\n"
|
||||
+ "\n".join(offenders),
|
||||
)
|
||||
|
||||
def test_late_resolution_refuses_the_published_config(self):
|
||||
from sglang.srt.arg_groups.overrides import declare_late_resolution
|
||||
from sglang.srt.runtime_context import get_context
|
||||
|
||||
Reference in New Issue
Block a user