Repository: caikit/caikit (python)

Issue:
DataObjectBase's __repr__ does not work properly 
## Describe the bug

When we build new class which is inherited from DataObjectBase, the new class could not inherit the correct DataObjectBase's __repr__ function.

## Platform

Please provide details about the environment you are using, including the following:

- Interpreter version:
- Library version:

## Sample Code

@dataobject(package="caikit_data_model.nlp")
class AggregatedSentiment(DataObjectBase):

## Expected behavior

The output of the new class should be the same as when we use watson_core, like
<img width="621" alt="image" src="https://github.com/caikit/caikit/assets/21294892/f2d6e8e7-a850-453e-bf1a-04c59e3d01d1">

## Observed behavior
<img width="1135" alt="image" src="https://github.com/caikit/caikit/assets/21294892/7031e0e8-2883-4d9b-b1c5-93afaa04ecdb">



## Additional context

Add any other context about the problem here.


Pull request description:
Don't overwrite the __repr__ with dataclass and fix HTTP output format
**What this PR does / why we need it**:

Closes #437 

This PR also stumbled on a fuuuun bug: We were never checking the output format of server-side streaming with SSE. Turns out it was just calling `__repr__` on each yielded data object which was returning as `"MyDataObject(foo=1)"` rather than the `json` version `{"foo": 1}`. This made the HTTP output stream virtually useless! Updating the base `__repr__` fixes this, so this PR also fixes the test for server side checking to make sure the output is `json` parsable.


Patch:
diff --git a/caikit/core/data_model/dataobject.py b/caikit/core/data_model/dataobject.py
index b6aa9971..8f793725 100644
--- a/caikit/core/data_model/dataobject.py
+++ b/caikit/core/data_model/dataobject.py
@@ -200,7 +200,7 @@ def dataobject(*args, **kwargs) -> Callable[[_DataObjectBaseT], _DataObjectBaseT
             if _has_dataclass_init(cls):
                 log.debug3("Resetting default dataclass init")
                 delattr(cls, "__init__")
-            cls = dataclasses.dataclass(cls)
+            cls = dataclasses.dataclass(repr=False)(cls)
             setattr(cls, _USER_DEFINED_DEFAULTS, user_defined_defaults)
 
         descriptor = _dataobject_to_proto(dataclass_=cls, **kwargs)
diff --git a/caikit/core/model_management/local_model_initializer.py b/caikit/core/model_management/local_model_initializer.py
index f6a00cce..d3d42680 100644
--- a/caikit/core/model_management/local_model_initializer.py
+++ b/caikit/core/model_management/local_model_initializer.py
@@ -188,7 +188,7 @@ class LocalModelInitializer(ModelInitializerBase):
                         **extra_kwargs,
                         **kwargs,
                     )
-                except Exception as err:  # pylint: disable=broad-exception-caught
+                except TypeError as err:
                     log.warning(
                         "<COR98539580W>",
                         "DEPRECATION: Loading %s failed with ModuleConfig. Using model_path. %s",
diff --git a/caikit/core/model_manager.py b/caikit/core/model_manager.py
index afc0a90c..7a00172a 100644
--- a/caikit/core/model_manager.py
+++ b/caikit/core/model_manager.py
@@ -203,7 +203,7 @@ class ModelManager:
 
     def load(
         self,
-        module_path: Union[str, BytesIO, bytes],
+        module_path: str,
         *,
         load_singleton: bool = False,
         finder: Union[str, ModelFinderBase] = "default",
@@ -251,9 +251,7 @@ class ModelManager:
         load_path = get_config().load_path
         if load_path is not None and isinstance(module_path, str):
             if not os.path.exists(module_path):
-                full_module_path = os.path.join(load_path, module_path)
-                if os.path.exists(full_module_path):
-                    module_path = full_module_path
+                module_path = os.path.join(load_path, module_path)
 
         # Ensure that we have a loadable directory.
         error.type_check("<COR98255419E>", str, BytesIO, bytes, module_path=module_path)
@@ -270,7 +268,7 @@ class ModelManager:
                 module_path, load_singleton, finder, initializer, **kwargs
             )
         try:
-            return self._do_load(
+            return self._load_from_dir(
                 module_path, load_singleton, finder, initializer, **kwargs
             )
         except FileNotFoundError:
@@ -422,7 +420,9 @@ class ModelManager:
 
     ## Implementation Details ##################################################
 
-    def _do_load(self, module_path, load_singleton, finder, initializer, **kwargs):
+    def _load_from_dir(
+        self, module_path, load_singleton, finder, initializer, **kwargs
+    ):
         """Load a model from a directory.
 
         Args:
@@ -517,7 +517,7 @@ class ModelManager:
             # to files directly, or it may unpack to a (single) directory containing the files.
             # We expect the former, but fall back to the second if we can't find the config.
             try:
-                model = self._do_load(
+                model = self._load_from_dir(
                     extract_path, load_singleton, finder, initializer, **kwargs
                 )
             # NOTE: Error handling is a little gross here, the main reason being that we
@@ -547,7 +547,7 @@ class ModelManager:
                 # Otherwise, try again. If we fail again stop, because the zip creation should only
                 # create one potential extra layer of nesting around the model directory.
                 try:
-                    model = self._do_load(
+                    model = self._load_from_dir(
                         nested_dirs[0], load_singleton, finder, initializer, **kwargs
                     )
                 except FileNotFoundError:
diff --git a/caikit/core/modules/base.py b/caikit/core/modules/base.py
index c37ac588..4f9fa468 100644
--- a/caikit/core/modules/base.py
+++ b/caikit/core/modules/base.py
@@ -40,7 +40,6 @@ from ..data_model import DataBase, DataStream
 from ..exceptions import error_handler
 from ..exceptions.validation_error import DataValidationError
 from ..toolkit import fileio
-from .config import ModuleConfig
 from .loader import ModuleLoader
 from .meta import _ModuleBaseMeta
 from caikit import core
@@ -121,19 +120,14 @@ class ModuleBase(metaclass=_ModuleBaseMeta):
 
     @classmethod
     @alog.logged_function(log.debug)
-    def load(
-        cls,
-        model_path: Union[str, ModuleConfig],
-        *args,
-        **kwargs,
-    ) -> "ModuleBase":
+    def load(cls, model_path, *args, **kwargs):
         """Load a new instance of workflow from a given model_path
 
         Args:
-            model_path (Union[str, ModuleConfig]): Path to saved model or
-                in-memory ModuleConfig
+            model_path (str): Path to workflow
         Returns:
-            model (ModuleBase): A new instance of this module class
+            caikit.core.workflows.base.WorkflowBase: A new instance of any given
+                implementation of WorkflowBase
         """
         return cls._load(ModuleLoader(model_path), *args, **kwargs)
 
@@ -183,7 +177,7 @@ class ModuleBase(metaclass=_ModuleBaseMeta):
     ###################
 
     @alog.logged_function(log.debug)
-    def save(self, model_path: str, *args, **kwargs):
+    def save(self, model_path, *args, **kwargs):
         """Save a model.
 
         Args:
@@ -195,7 +189,7 @@ class ModuleBase(metaclass=_ModuleBaseMeta):
         )
 
     @alog.logged_function(log.debug)
-    def as_file_like_object(self, *args, **kwargs) -> BytesIO:
+    def as_file_like_object(self, *args, **kwargs):
         """Produces a file-like object corresponding to a zip archive affiliated with a given
         model. This method wraps is functionally similar to .save() - it saves a model into
         a temporary directory and produces a zip archive, then loads the result as a io.BytesIO
@@ -211,7 +205,7 @@ class ModuleBase(metaclass=_ModuleBaseMeta):
         return BytesIO(self.as_bytes(*args, **kwargs))
 
     @alog.logged_function(log.debug)
-    def as_bytes(self, *args, **kwargs) -> bytes:
+    def as_bytes(self, *args, **kwargs):
         """Produces a bytes object corresponding to a zip archive affiliated with a given
         model. This method wraps is functionally similar to .save() - it saves a model into
         a temporary directory and produces a zip archive, then loads the result as a bytes
@@ -683,7 +677,7 @@ class ModuleBase(metaclass=_ModuleBaseMeta):
             NotImplementedError("This is not available in this module."),
         )
 
-    def _extract_pred_set(self, dataset, preprocess_func=None, **kwargs):
+    def _extract_pred_set(self, dataset, *args, preprocess_func=None, **kwargs):
         """Method for extracting pred set from dataset. Implemented in subclass.
 
         Args:
@@ -693,7 +687,7 @@ class ModuleBase(metaclass=_ModuleBaseMeta):
                 steps that need to be taken to run the model on the input text.
                 This helper function ultimately leads to the input to this
                 module and may involve executing other modules.
-            **kwargs (dict): Optional keyword arguments for prediction set extraction.
+            *args, **kwargs (dict): Optional keyword arguments for prediction set extraction.
         Returns:
             list: List of labels in the format of the module_type that is being
                 called.
diff --git a/caikit/runtime/http_server.py b/caikit/runtime/http_server.py
index a848786d..56adb227 100644
--- a/caikit/runtime/http_server.py
+++ b/caikit/runtime/http_server.py
@@ -18,17 +18,7 @@ API based on the task definitions available at boot.
 """
 # Standard
 from functools import partial
-from typing import (
-    Any,
-    Dict,
-    Iterable,
-    List,
-    Optional,
-    Type,
-    Union,
-    get_args,
-    get_type_hints,
-)
+from typing import Any, Dict, Iterable, List, Optional, Type, Union, get_args
 import asyncio
 import enum
 import json
@@ -656,32 +646,26 @@ class RuntimeHTTPServer(RuntimeServerBase):
         """Make a pydantic model based on the given proto message by using the data
         model class annotations to mirror as a pydantic model
         """
-        # define a local namespace for type hints to get type information from.
-        # This is needed for pydantic to have a handle on JsonDict and JsonDictValue while
+        # Use dicts for jsondict values
+        # this is needed for pydantic to have a handle on JsonDicts while
         # creating its base model
-        localns = {"JsonDict": dict, "JsonDictValue": dict}
+        # pylint: disable=unused-variable
+        JsonDict = dict
+        JsonDictValue = dict
 
         if dm_class in PYDANTIC_TO_DM_MAPPING:
             return PYDANTIC_TO_DM_MAPPING[dm_class]
 
         annotations = {
             field_name: cls._get_pydantic_type(field_type)
-            for field_name, field_type in get_type_hints(
-                dm_class, localns=localns
-            ).items()
+            for field_name, field_type in dm_class.__annotations__.items()
         }
         pydantic_model = type(pydantic.BaseModel)(
             dm_class.get_proto_class().DESCRIPTOR.full_name,
             (pydantic.BaseModel,),
             {
                 "__annotations__": annotations,
-                **{
-                    name: None
-                    for name, _ in get_type_hints(
-                        dm_class,
-                        localns=localns,
-                    ).items()
-                },
+                **{name: None for name in dm_class.__annotations__},
             },
         )
         PYDANTIC_TO_DM_MAPPING[dm_class] = pydantic_model
diff --git a/caikit/runtime/model_management/model_loader.py b/caikit/runtime/model_management/model_loader.py
index 2a3c3996..ab8807c8 100644
--- a/caikit/runtime/model_management/model_loader.py
+++ b/caikit/runtime/model_management/model_loader.py
@@ -142,7 +142,7 @@ class ModelLoader:
                 % (model_path, repr(ex)),
                 "model_id": model_id,
             }
-            log.error(log_dict, exc_info=True)
+            log.error(log_dict)
             raise CaikitRuntimeException(
                 StatusCode.INTERNAL,
                 f"Model {model_id} failed to load. Nested error: {ex}",
diff --git a/pyproject.toml b/pyproject.toml
index 8007ed2b..798808ce 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -43,7 +43,7 @@ runtime-grpc = [
 ]
 
 runtime-http = [
-    "fastapi[all]>=0.95,<1",
+    "fastapi[all]>=0.100,<1",
     "sse-starlette>=1.6.1,<2",
 ]
 


Test patch:
diff --git a/tests/core/data_model/test_dataobject.py b/tests/core/data_model/test_dataobject.py
index f483cd49..2ba5c035 100644
--- a/tests/core/data_model/test_dataobject.py
+++ b/tests/core/data_model/test_dataobject.py
@@ -1129,3 +1129,29 @@ def test_make_dataobject_with_optionals():
     assert data_object.prop == "val"
     assert data_object._proto_class.DESCRIPTOR.name == "SomeOtherFooBar"
     assert data_object._proto_class.DESCRIPTOR.file.package == "foo.bar.baz"
+
+
+def test_dataobject_repr_data_base():
+    """Make sure that the __repr__ on a dataobject uses DataBase.__repr__"""
+
+    @dataobject
+    class Foo(DataObjectBase):
+        foo: int
+
+    inst = Foo(1)
+    assert repr(inst) == DataBase.__repr__(inst)
+
+
+def test_dataobject_custom_repr():
+    """Make sure that a dataobject with a custom __repr__ retains it"""
+    custom_repr = "so custom!"
+
+    @dataobject
+    class Foo(DataObjectBase):
+        foo: int
+
+        def __repr__(self):
+            return custom_repr
+
+    inst = Foo(1)
+    assert repr(inst) == custom_repr
diff --git a/tests/core/test_model_manager.py b/tests/core/test_model_manager.py
index b9a003d1..4bc63eb8 100644
--- a/tests/core/test_model_manager.py
+++ b/tests/core/test_model_manager.py
@@ -18,13 +18,11 @@ and download and load them.
 from contextlib import contextmanager
 import os
 import tempfile
-import uuid
 
 # Local
 from caikit.core import LocalBackend
 from caikit.core.data_model import DataStream, TrainingStatus
 from caikit.core.model_management import ModelFinderBase, model_finder_factory
-from caikit.core.modules import ModuleBase, ModuleSaver, module
 
 # Unit Test Infrastructure
 from sample_lib.modules.sample_task import SampleModule
@@ -602,7 +600,7 @@ def test_initialize_all_components(reset_globals):
         assert set(MODEL_MANAGER._initializers.keys()) == {"default", "foobar"}
 
 
-def test_find_without_on_disk_model_no_load_path(good_model_path, reset_globals):
+def test_find_without_on_disk_model(good_model_path, reset_globals):
     """Make sure that ephemeral models can be found without existing on disk"""
 
     with temp_config(
@@ -618,53 +616,6 @@ def test_find_without_on_disk_model_no_load_path(good_model_path, reset_globals)
         assert not os.path.exists("some_pretend_model")
 
 
-def test_find_without_on_disk_model_and_load_path(good_model_path, reset_globals):
-    """Make sure that ephemeral models can be found without existing on disk
-    when a shared load_path is configured
-    """
-    with tempfile.TemporaryDirectory() as workdir:
-        with temp_config(
-            {
-                "load_path": workdir,
-                "model_management": {
-                    "finders": {"default": {"type": TestFinder.name}},
-                    "initializers": {"default": {"type": "LOCAL"}},
-                },
-            }
-        ):
-            model = caikit.core.load("some_pretend_model")
-            assert model
-            assert not os.path.exists("some_pretend_model")
-
-
-def test_load_requires_path_str(reset_globals):
-    """Make sure that modules whose load function requires a path string
-    explicitly can be loaded
-    """
-    mod_id = str(uuid.uuid4())
-
-    class CustomException(Exception):
-        """Never been seen before!"""
-
-    @module(id=mod_id, name="Needs a path", version="1.2.3")
-    class NeedsAPath(ModuleBase):
-        def save(self, model_path: str):
-            with ModuleSaver(self, model_path=model_path) as module_saver:
-                module_saver.update_config({"foo": "bar"})
-
-        @classmethod
-        def load(cls, model_path: str, *_, **__):
-            if not isinstance(model_path, str) or not os.path.exists(model_path):
-                raise CustomException("Yikes!")
-            return cls()
-
-    with tempfile.TemporaryDirectory() as workdir:
-        model_path = os.path.join(workdir, "test_model")
-        NeedsAPath().save(model_path)
-        loaded = caikit.load(model_path)
-        assert loaded
-
-
 def test_train_by_module_class(reset_globals):
     """Make sure training can be accessed through the central train function
     with the class directly
diff --git a/tests/runtime/test_http_server.py b/tests/runtime/test_http_server.py
index 9625b29c..6034c219 100644
--- a/tests/runtime/test_http_server.py
+++ b/tests/runtime/test_http_server.py
@@ -373,12 +373,17 @@ def test_inference_streaming_sample_module(sample_task_model_id, runtime_http_se
             json=json_input,
         )
         assert stream.status_code == 200
-        assert (
-            stream.content.decode(stream.default_encoding).count("SampleOutputType")
-            == 10
+        stream_content = stream.content.decode(stream.default_encoding)
+        stream_responses = json.loads(
+            "[{}]".format(
+                stream_content.replace("data: ", "")
+                .replace("\r\n", "")
+                .replace("}{", "}, {")
+            )
         )
-        assert (
-            b"SampleOutputType(greeting='Hello world stream')\r\n\r\n" in stream.content
+        assert len(stream_responses) == 10
+        assert all(
+            resp.get("greeting") == "Hello world stream" for resp in stream_responses
         )
 
 
@@ -429,34 +434,7 @@ def test_pydantic_wrapping_with_enums():
     assert token.text == "foo"
 
 
-def test_pydantic_wrapping_with_inheritance():
-    """Check that the pydantic wrapping works on our data models when they involve inheritance"""
-
-    @dataobject
-    class Base(DataObjectBase):
-        foo: int
-        bar: int
-
-    @dataobject
-    class Derived(Base):
-        bar: str
-        baz: str
-
-    # Check that our data model is fine and dandy
-    derived = Derived(bar="one", foo=2)
-    assert derived.bar == "one"
-    assert derived.foo == 2
-
-    # Wrap the containing data model in pydantic
-    pydantic_datamodel = http_server.RuntimeHTTPServer._dataobject_to_pydantic(Derived)
-
-    # Check that our data model is _still_ fine and dandy
-    new_derived = pydantic_datamodel(bar="one", foo=2)
-    assert new_derived.bar == "one"
-    assert new_derived.foo == 2
-
-
-def test_pydantic_wrapping_with_lists():
+def test_pydantic_wrapping_with_lists(runtime_http_server):
     """Check that pydantic wrapping works on data models with lists"""
 
     @dataobject(package="http")
@@ -470,7 +448,7 @@ def test_pydantic_wrapping_with_lists():
     foo = FooTest(bars=[BarTest(1)])
     assert foo.bars[0].baz == 1
 
-    http_server.RuntimeHTTPServer._dataobject_to_pydantic(FooTest)
+    runtime_http_server._dataobject_to_pydantic(FooTest)
 
     foo = FooTest(bars=[BarTest(1)])
     assert foo.bars[0].baz == 1