Error message improvements

internal/server/runner.go

@@ -339,7 +339,7 @@ func (r *Runner) wait() {
 	err := r.cmd.Wait()
 	if err != nil {
 		runnerLogs := r.rotateLogs()
-		log.Errorw("python runner exited with error", "pid", r.cmd.Process.Pid, "error", err, "logs", runnerLogs)
+		log.Errorw("python runner exited with error", "pid", r.cmd.Process.Pid, "error", err)
 		for _, pr := range r.pending {
 			pr.mu.Lock()
 			now := util.NowIso()

python/coglet/inspector.py

@@ -14,15 +14,17 @@ def _check_parent(child: type, parent: type) -> bool:
 
 
 def _validate_setup(f: Callable) -> None:
-    assert inspect.isfunction(f), f'not a function {f}'
+    assert inspect.isfunction(f), 'setup is not a function'
     spec = inspect.getfullargspec(f)
+
+    assert spec.args[:1] == ['self'], "setup() must have 'self' first argument"
     non_default_parameter_args = spec.args
     if spec.defaults is not None:
         non_default_parameter_args = non_default_parameter_args[: -len(spec.defaults)]
-    assert non_default_parameter_args == ['self'] or non_default_parameter_args == [
-        'self',
-        'weights',
-    ], f'unexpected setup() arguments: {non_default_parameter_args}'
+    extra_args = ', '.join(
+        [a for a in non_default_parameter_args if a not in {'self', 'weights'}]
+    )
+    assert extra_args == '', f'unexpected setup() arguments: {extra_args}'
     assert spec.varargs is None, 'setup() must not have *args'
     assert spec.varkw is None, 'setup() must not have **kwargs'
     assert spec.kwonlyargs == [], 'setup() must not have keyword-only args'
@@ -31,11 +33,11 @@ def _validate_setup(f: Callable) -> None:
 
 
 def _validate_predict(f: Callable, is_class_fn: bool) -> None:
-    assert inspect.isfunction(f), f'not a function: {f}'
+    assert inspect.isfunction(f), 'predict is not a function'
     spec = inspect.getfullargspec(f)
 
     if is_class_fn:
-        assert spec.args[0] == 'self', "predict() must have 'self' first argument"
+        assert spec.args[:1] == ['self'], "predict() must have 'self' first argument"
     assert spec.varargs is None, 'predict() must not have *args'
     assert spec.varkw is None, 'predict() must not have **kwargs'
     assert spec.kwonlyargs == [], 'predict() must not have keyword-only args'

python/tests/errors/cog.yaml

@@ -0,0 +1 @@
+predict: "predict.py:Predictor"

python/tests/errors/predict_fn.py

@@ -0,0 +1,10 @@
+from cog import BasePredictor
+
+ERROR = 'predict is not a function'
+
+
+class Predictor(BasePredictor):
+    def setup(self) -> None:
+        pass
+
+    predict = 0

python/tests/errors/predict_kwarg.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = 'predict() must not have **kwargs'
+
+
+class Predictor(BasePredictor):
+    def setup(self) -> None:
+        pass
+
+    def predict(self, **kwargs) -> None:
+        pass

python/tests/errors/predict_none.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = 'predict() must not return None'
+
+
+class Predictor(BasePredictor):
+    def setup(self) -> None:
+        pass
+
+    def predict(self, s: str) -> None:
+        pass

python/tests/errors/predict_self.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = "predict() must have 'self' first argument"
+
+
+class Predictor(BasePredictor):
+    def setup(self) -> None:
+        pass
+
+    def predict() -> str:
+        return ''

python/tests/errors/predict_vararg.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = 'predict() must not have *args'
+
+
+class Predictor(BasePredictor):
+    def setup(self) -> None:
+        pass
+
+    def predict(self, *args) -> None:
+        pass

python/tests/errors/setup_arg.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = 'unexpected setup() arguments: x, y'
+
+
+class Predictor(BasePredictor):
+    def setup(self, x: int, y: int) -> None:
+        pass
+
+    def predict(self, s: str) -> str:
+        return s

python/tests/errors/setup_fn.py

@@ -0,0 +1,10 @@
+from cog import BasePredictor
+
+ERROR = 'setup is not a function'
+
+
+class Predictor(BasePredictor):
+    setup = 0
+
+    def predict(self, s: str) -> str:
+        return s

python/tests/errors/setup_kwargs.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = 'setup() must not have **kwargs'
+
+
+class Predictor(BasePredictor):
+    def setup(self, **kwargs) -> None:
+        pass
+
+    def predict(self, s: str) -> str:
+        return s

python/tests/errors/setup_none.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = 'setup() must return None'
+
+
+class Predictor(BasePredictor):
+    def setup(self) -> int:
+        return 0
+
+    def predict(self, s: str) -> str:
+        return s

python/tests/errors/setup_self.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = "setup() must have 'self' first argument"
+
+
+class Predictor(BasePredictor):
+    def setup() -> None:
+        pass
+
+    def predict(self, s: str) -> str:
+        return s

python/tests/errors/setup_varargs.py

@@ -0,0 +1,11 @@
+from cog import BasePredictor
+
+ERROR = 'setup() must not have *args'
+
+
+class Predictor(BasePredictor):
+    def setup(self, *args) -> None:
+        pass
+
+    def predict(self, s: str) -> str:
+        return s

python/tests/test_errors.py

@@ -0,0 +1,27 @@
+import importlib
+import os.path
+import pkgutil
+import re
+from typing import List
+
+import pytest
+
+from coglet import inspector
+
+
+def get_predictors() -> List[str]:
+    errors_dir = os.path.join(os.path.dirname(__file__), 'errors')
+    return [name for _, name, _ in pkgutil.iter_modules([errors_dir])]
+
+
+def run_error(module_name: str, predictor_name: str) -> None:
+    m = importlib.import_module(module_name)
+    err_msg = getattr(m, 'ERROR')
+    with pytest.raises(AssertionError, match=re.escape(err_msg)):
+        inspector.create_predictor(module_name, predictor_name)
+
+
+@pytest.mark.parametrize('predictor', get_predictors())
+def test_predictor(predictor):
+    module_name = f'tests.errors.{predictor}'
+    run_error(module_name, 'Predictor')

script/check.sh

@@ -27,7 +27,11 @@ check_python() {
         uv tool run ruff check
         uv tool run ruff format --check
     fi
-    .venv/bin/mypy . --exclude build --exclude python/tests/cases --exclude python/tests/runners --exclude python/tests/schemas
+    .venv/bin/mypy . --exclude build \
+        --exclude python/tests/cases \
+        --exclude python/tests/errors \
+        --exclude python/tests/runners \
+        --exclude python/tests/schemas
 }
 
 if [ $# -eq 0 ]; then