From e19b95f30bc6cd8a37179cd4c58d85fd20e216ef Mon Sep 17 00:00:00 2001 From: Siddhant Date: Sun, 27 Sep 2026 12:43:00 +0530 Subject: [PATCH 1/4] fix: validate all models before loading --- src/tserve/runtime/bootstrap.py | 25 ++++++++++++---------- src/tserve/runtime/tests/test_bootstrap.py | 23 +++++++++++++++++++- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/tserve/runtime/bootstrap.py b/src/tserve/runtime/bootstrap.py index 6f8e172..61d2c40 100644 --- a/src/tserve/runtime/bootstrap.py +++ b/src/tserve/runtime/bootstrap.py @@ -89,11 +89,10 @@ def loaded_models(self) -> ModelsResult: def bootstrap(model: list[str | Path | tuple[str, Any]]) -> Runtime: """Resolve, construct, load, warmup, and register each selected model. - For every item: ``resolve_model`` → ``create_executor(info.executor)`` - → ``load`` → ``warmup`` → ``stats.register``. Tuple items pass the - second element (craft spec or object) to ``load``; strings and - paths pass the item itself. Duplicate ``ModelInfo.id`` values raise - before a second load. + Resolve and check all items before creating any executor. Then, for + every item: ``create_executor(info.executor)`` → ``load`` → ``warmup`` + → ``stats.register``. Tuple items pass the second element (craft spec + or object) to ``load``; strings and paths pass the item itself. Parameters ---------- @@ -132,6 +131,15 @@ def bootstrap(model: list[str | Path | tuple[str, Any]]) -> Runtime: tserve.scheduling.scheduler.Scheduler Forecast dispatch is not implemented in this package. """ + resolved: list[tuple[ModelInfo, Any]] = [] + seen: set[str] = set() + for item in model: + info = resolve_model(item) + if info.id in seen: + raise ValueError(f"duplicate model id {info.id!r} in model") + seen.add(info.id) + resolved.append((info, item[1] if isinstance(item, tuple) else item)) + stats = Stats() executors: dict[str, Executor] = {} models: dict[str, ModelInfo] = {} @@ -145,12 +153,7 @@ def bootstrap(model: list[str | Path | tuple[str, Any]]) -> Runtime: f"{paint('is always loaded as a baseline', '2')}" ) - for position, item in enumerate(model, start=1): - info = resolve_model(item) - item = item[1] if isinstance(item, tuple) else item - - if info.id in models: - raise ValueError(f"duplicate model id {info.id!r} in model") + for position, (info, item) in enumerate(resolved, start=1): label = f"{info.id} via {info.executor}" dots = paint("." * max(3, 40 - len(label)), "2") diff --git a/src/tserve/runtime/tests/test_bootstrap.py b/src/tserve/runtime/tests/test_bootstrap.py index 9b1d394..ce1f7f5 100644 --- a/src/tserve/runtime/tests/test_bootstrap.py +++ b/src/tserve/runtime/tests/test_bootstrap.py @@ -77,10 +77,31 @@ def test_bootstrap_loads_craft(): def test_bootstrap_rejects_duplicate(): with ( patch("tserve.runtime.bootstrap.resolve_model", return_value=_info()), - patch("tserve.runtime.bootstrap.create_executor", return_value=_executor()), + patch("tserve.runtime.bootstrap.create_executor") as create_executor, pytest.raises(ValueError, match="duplicate model id 'naive' in model"), ): bootstrap(["naive", "naive"]) + create_executor.assert_not_called() + + +def test_bootstrap_validates_before_loading(): + executor = _executor() + with ( + patch( + "tserve.runtime.bootstrap.resolve_model", + side_effect=[_info(), ValueError("unknown model 'missing'")], + ) as resolve_model, + patch( + "tserve.runtime.bootstrap.create_executor", return_value=executor + ) as create_executor, + pytest.raises(ValueError, match="unknown model 'missing'"), + ): + bootstrap(["naive", "missing"]) + + assert resolve_model.call_count == 2 + create_executor.assert_not_called() + executor.load.assert_not_called() + executor.warmup.assert_not_called() def test_loaded_models(): From 99e0246066d6db58e8db9b07eaa2a77ccc9b220b Mon Sep 17 00:00:00 2001 From: Siddhant Date: Sun, 27 Sep 2026 12:43:11 +0530 Subject: [PATCH 2/4] fix: report invalid CLI models without traceback --- src/tserve/cli/main.py | 22 ++++++++++------------ src/tserve/cli/tests/test_main.py | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/tserve/cli/main.py b/src/tserve/cli/main.py index b9d6a91..bc9b89e 100644 --- a/src/tserve/cli/main.py +++ b/src/tserve/cli/main.py @@ -121,11 +121,6 @@ def main(argv: list[str] | None = None) -> int: ------ SystemExit From ``ArgumentParser.error`` or argparse usage errors. - ValueError - If positional tokens are malformed (empty ``id=spec``, - a bare craft spec), or ``Server`` / ``bootstrap`` reject a - load spec (duplicate id, unknown registry id, non-zip path, - unknown executor). TypeError If a model object is not a sktime ``BaseForecaster``. ImportError @@ -150,13 +145,16 @@ def main(argv: list[str] | None = None) -> int: from tserve.server import Server - server = Server( - model=parse_model(args.positional_model), - models_dir=args.models_dir, - host=args.host, - port=args.port, - log_level=args.log_level, - ) + try: + server = Server( + model=parse_model(args.positional_model), + models_dir=args.models_dir, + host=args.host, + port=args.port, + log_level=args.log_level, + ) + except ValueError as exc: + parser.error(str(exc)) try: server.run() except KeyboardInterrupt: diff --git a/src/tserve/cli/tests/test_main.py b/src/tserve/cli/tests/test_main.py index 250d3f0..40d64db 100644 --- a/src/tserve/cli/tests/test_main.py +++ b/src/tserve/cli/tests/test_main.py @@ -107,3 +107,18 @@ def test_main_version(capsys): assert excinfo.value.code == 0 assert capsys.readouterr().out.strip() == f"tserve {__version__}" + + +def test_main_reports_invalid_model_without_traceback(capsys): + with ( + patch( + "tserve.server.Server", side_effect=ValueError("unknown model 'missing'") + ), + pytest.raises(SystemExit) as excinfo, + ): + main(["missing"]) + + assert excinfo.value.code == 2 + err = capsys.readouterr().err + assert "unknown model 'missing'" in err + assert "Traceback" not in err From 4d3009d0d0f828d87d9c9d46ec8da14115333764 Mon Sep 17 00:00:00 2001 From: Siddhant Date: Mon, 28 Sep 2026 14:12:23 +0530 Subject: [PATCH 3/4] style: apply Ruff formatting to bootstrap --- src/tserve/runtime/bootstrap.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tserve/runtime/bootstrap.py b/src/tserve/runtime/bootstrap.py index 61d2c40..3ae413c 100644 --- a/src/tserve/runtime/bootstrap.py +++ b/src/tserve/runtime/bootstrap.py @@ -154,7 +154,6 @@ def bootstrap(model: list[str | Path | tuple[str, Any]]) -> Runtime: ) for position, (info, item) in enumerate(resolved, start=1): - label = f"{info.id} via {info.executor}" dots = paint("." * max(3, 40 - len(label)), "2") prefix = ( From 869dc28be67662ea7f9b041480dccb8665c689bf Mon Sep 17 00:00:00 2001 From: Siddhant Date: Fri, 2 Oct 2026 03:37:30 +0530 Subject: [PATCH 4/4] fix: remove CLI error handling from model validation PR --- src/tserve/cli/main.py | 22 ++++++++++++---------- src/tserve/cli/tests/test_main.py | 15 --------------- 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/tserve/cli/main.py b/src/tserve/cli/main.py index bc9b89e..b9d6a91 100644 --- a/src/tserve/cli/main.py +++ b/src/tserve/cli/main.py @@ -121,6 +121,11 @@ def main(argv: list[str] | None = None) -> int: ------ SystemExit From ``ArgumentParser.error`` or argparse usage errors. + ValueError + If positional tokens are malformed (empty ``id=spec``, + a bare craft spec), or ``Server`` / ``bootstrap`` reject a + load spec (duplicate id, unknown registry id, non-zip path, + unknown executor). TypeError If a model object is not a sktime ``BaseForecaster``. ImportError @@ -145,16 +150,13 @@ def main(argv: list[str] | None = None) -> int: from tserve.server import Server - try: - server = Server( - model=parse_model(args.positional_model), - models_dir=args.models_dir, - host=args.host, - port=args.port, - log_level=args.log_level, - ) - except ValueError as exc: - parser.error(str(exc)) + server = Server( + model=parse_model(args.positional_model), + models_dir=args.models_dir, + host=args.host, + port=args.port, + log_level=args.log_level, + ) try: server.run() except KeyboardInterrupt: diff --git a/src/tserve/cli/tests/test_main.py b/src/tserve/cli/tests/test_main.py index 40d64db..250d3f0 100644 --- a/src/tserve/cli/tests/test_main.py +++ b/src/tserve/cli/tests/test_main.py @@ -107,18 +107,3 @@ def test_main_version(capsys): assert excinfo.value.code == 0 assert capsys.readouterr().out.strip() == f"tserve {__version__}" - - -def test_main_reports_invalid_model_without_traceback(capsys): - with ( - patch( - "tserve.server.Server", side_effect=ValueError("unknown model 'missing'") - ), - pytest.raises(SystemExit) as excinfo, - ): - main(["missing"]) - - assert excinfo.value.code == 2 - err = capsys.readouterr().err - assert "unknown model 'missing'" in err - assert "Traceback" not in err