fix: Feedback from Code Rabbit. Excellent as usual.

Author: grahamking

examples/cli/cli.py

@@ -79,15 +79,14 @@ async def run(runtime: DistributedRuntime):
     input = args.input_source
     output = args.output_type
 
-    if output == "echo":
-        engine_type = EngineType.Echo
-    elif output == "mistralrs":
-        engine_type = EngineType.MistralRs
-    elif output == "llamacpp":
-        engine_type = EngineType.LlamaCpp
-    elif output == "dyn":
-        engine_type = EngineType.Dynamic
-    else:
+    engine_type_map = {
+        "echo": EngineType.Echo,
+        "mistralrs": EngineType.MistralRs,
+        "llamacpp": EngineType.LlamaCpp,
+        "dyn": EngineType.Dynamic,
+    }
+    engine_type = engine_type_map.get(output)
+    if engine_type is None:
         print(f"Unsupported output type: {output}")
         sys.exit(1)
 

lib/bindings/python/README.md

@@ -46,7 +46,7 @@ uv pip install maturin
 maturin develop --uv
 ```
 
-5. Experimental: To allow using mistral.rs and llama.cpp via the bindings, build with feature flags
+5. Experimental: To allow using mistral.rs and llama.cpp via the bindings, build with feature flags:
 
 ```
 maturin develop --features mistralrs,llamacpp
@@ -61,7 +61,7 @@ the stub `libcuda.so` is earlier on the library search path than the real libcud
 patchelf --set-rpath '' _core.cpython-312-x86_64-linux-gnu.so
 ```
 
-If you include `llamacpp` feature flag, `libllama.so` and `libggml.so` (and family) will need to be available at runtime.
+If you include the `llamacpp` feature flag, `libllama.so` and `libggml.so` (and family) will need to be available at runtime.
 
 
 ## Run Examples

lib/bindings/python/rust/llm/entrypoint.rs

@@ -3,6 +3,7 @@
 
 use std::fmt::Display;
 use std::path::PathBuf;
+use std::sync::Arc;
 
 use pyo3::{exceptions::PyException, prelude::*};
 
@@ -52,9 +53,16 @@ impl EntrypointArgs {
         //router_config: Option<RouterConfig>,
         kv_cache_block_size: Option<u32>,
         http_port: Option<u16>,
-    ) -> Self {
-        let endpoint_id_obj: Option<EndpointId> = endpoint_id.and_then(|eid| eid.parse().ok());
-        EntrypointArgs {
+    ) -> PyResult<Self> {
+        let endpoint_id_obj: Option<EndpointId> = match endpoint_id {
+            Some(eid) => Some(eid.parse().map_err(|_| {
+                PyErr::new::<pyo3::exceptions::PyValueError, _>(format!(
+                    "Invalid endpoint_id format: {eid}"
+                ))
+            })?),
+            None => None,
+        };
+        Ok(EntrypointArgs {
             engine_type,
             model_path,
             model_name,
@@ -65,14 +73,14 @@ impl EntrypointArgs {
             //router_config,
             kv_cache_block_size,
             http_port,
-        }
+        })
     }
 }
 
 #[pyclass]
 #[derive(Clone)]
 pub(crate) struct EngineConfig {
-    inner: RsEngineConfig,
+    inner: Arc<RsEngineConfig>,
 }
 
 #[pyfunction]
@@ -97,7 +105,9 @@ pub fn make_engine<'p>(
         let inner = select_engine(distributed_runtime, args.engine_type, local_model)
             .await
             .map_err(to_pyerr)?;
-        Ok(EngineConfig { inner })
+        Ok(EngineConfig {
+            inner: Arc::new(inner),
+        })
     })
 }
 
@@ -154,7 +164,6 @@ async fn select_engine(
     Ok(inner)
 }
 
-// TODO input should be an enum
 #[pyfunction]
 #[pyo3(signature = (distributed_runtime, input, engine_config))]
 pub fn run_input<'p>(
@@ -164,11 +173,16 @@ pub fn run_input<'p>(
     engine_config: EngineConfig,
 ) -> PyResult<Bound<'p, PyAny>> {
     let input_enum: Input = input.parse().map_err(to_pyerr)?;
+    let Some(engine_config) = Arc::into_inner(engine_config.inner) else {
+        return Err(to_pyerr(anyhow::anyhow!(
+            "EngineConfig Arc has been cloned. Should be impossible."
+        )));
+    };
     pyo3_async_runtimes::tokio::future_into_py(py, async move {
         dynamo_llm::entrypoint::input::run_input(
             either::Either::Right(distributed_runtime.inner.clone()),
             input_enum,
-            engine_config.inner,
+            engine_config,
         )
         .await
         .map_err(to_pyerr)?;

lib/bindings/python/src/dynamo/_core.pyi

@@ -787,10 +787,6 @@ async def register_llm(model_type: ModelType, endpoint: Endpoint, model_path: st
     """Attach the model at path to the given endpoint, and advertise it as model_type"""
     ...
 
-class EngineType:
-    """Enum saying which backend to us: Echo, MistralRs, Dyn, etc."""
-    ...
-
 class EngineConfig:
     """Holds internal configuration for a Dynamo engine."""
     ...
@@ -799,7 +795,7 @@ async def make_engine(args: EntrypointArgs) -> EngineConfig:
     """Make an engine matching the args"""
     ...
 
-async def run_input(input: str, runtime: Runtime, engine_config: EngineConfig) -> None:
+async def run_input(runtime: DistributedRuntime, input: str, engine_config: EngineConfig) -> None:
     """Start an engine, connect it to an input, and run until stopped."""
     ...
 

lib/bindings/python/src/dynamo/llm/__init__.py

@@ -1,17 +1,7 @@
 # SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 # SPDX-License-Identifier: Apache-2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
+
+# flake8: noqa
 
 import logging