diff --git a/sdk/tests/conftest.py b/sdk/tests/conftest.py index e6814dfcb..e62bd871e 100644 --- a/sdk/tests/conftest.py +++ b/sdk/tests/conftest.py @@ -16,7 +16,12 @@ from turing.router.config.resource_request import ResourceRequest from turing.router.config.log_config import LogConfig, ResultLoggerType from turing.router.config.enricher import Enricher -from turing.router.config.router_ensembler_config import DockerRouterEnsemblerConfig +from turing.router.config.router_ensembler_config import ( + EnsemblerNopConfig, + EnsemblerStandardConfig, + DockerRouterEnsemblerConfig, + PyfuncRouterEnsemblerConfig +) from turing.router.config.common.env_var import EnvVar from turing.router.config.experiment_config import ExperimentConfig from tests.fixtures.mlflow import mock_mlflow @@ -80,6 +85,58 @@ def generic_ensemblers(project, num_ensemblers): updated_at=datetime.now() + timedelta(seconds=i + 10) ) for i in range(1, num_ensemblers + 1)] +@pytest.fixture +def nop_router_ensembler_config(): + return EnsemblerNopConfig(final_response_route_id="test") + +@pytest.fixture +def standard_router_ensembler_config(): + return EnsemblerStandardConfig( + experiment_mappings=[ + turing.generated.models.EnsemblerStandardConfigExperimentMappings( + experiment="experiment-1", + treatment="treatment-1", + route="route-1" + ), + turing.generated.models.EnsemblerStandardConfigExperimentMappings( + experiment="experiment-2", + treatment="treatment-2", + route="route-2" + ) + ], + fallback_response_route_id="route-1" + ) + +@pytest.fixture +def docker_router_ensembler_config(): + return DockerRouterEnsemblerConfig( + image="test.io/just-a-test/turing-ensembler:0.0.0-build.0", + resource_request=ResourceRequest( + min_replica=1, + max_replica=3, + cpu_request="500m", + memory_request="512Mi" + ), + endpoint=f"http://localhost:5000/ensembler_endpoint", + timeout="500ms", + port=5120, + env=[], + ) + +@pytest.fixture +def pyfunc_router_ensembler_config(): + return PyfuncRouterEnsemblerConfig( + project_id=1, + ensembler_id=1, + resource_request=ResourceRequest( + min_replica=1, + max_replica=3, + cpu_request="500m", + memory_request="512Mi" + ), + timeout="500ms", + env=[], + ) @pytest.fixture def bucket_name(): @@ -469,7 +526,7 @@ def generic_router_version( @pytest.fixture -def generic_router_config(): +def generic_router_config(docker_router_ensembler_config): return RouterConfig( environment_name="id-dev", name="router-1", @@ -530,19 +587,7 @@ def generic_router_config(): ) ] ), - ensembler=DockerRouterEnsemblerConfig( - image="test.io/just-a-test/turing-ensembler:0.0.0-build.0", - resource_request=ResourceRequest( - min_replica=1, - max_replica=3, - cpu_request="500m", - memory_request="512Mi" - ), - endpoint=f"http://localhost:5000/ensembler_endpoint", - timeout="500ms", - port=5120, - env=[], - ) + ensembler=docker_router_ensembler_config ) diff --git a/sdk/tests/router/config/router_config_test.py b/sdk/tests/router/config/router_config_test.py index a4b482df0..def9b812e 100644 --- a/sdk/tests/router/config/router_config_test.py +++ b/sdk/tests/router/config/router_config_test.py @@ -1,6 +1,13 @@ import pytest from turing.router.config.route import Route, DuplicateRouteException, InvalidRouteException - +from turing.router.config.resource_request import ResourceRequest +from turing.router.config.router_ensembler_config import ( + DockerRouterEnsemblerConfig, + NopRouterEnsemblerConfig, + StandardRouterEnsemblerConfig, + PyfuncRouterEnsemblerConfig, + RouterEnsemblerConfig +) @pytest.mark.parametrize( "actual,new_routes,expected", [ @@ -29,6 +36,7 @@ def test_set_router_config_with_invalid_routes(actual, new_routes, expected, req ]) def test_set_router_config_with_invalid_default_route(actual, invalid_route_id, expected, request): actual = request.getfixturevalue(actual) + actual.ensembler = None actual.default_route_id = invalid_route_id with pytest.raises(expected): actual.to_open_api() @@ -43,3 +51,76 @@ def test_set_router_config_with_invalid_default_route(actual, invalid_route_id, def test_remove_router_config_default_route(actual, expected, request): actual = request.getfixturevalue(actual) assert "default_route_id" not in actual.to_open_api() + +@pytest.mark.parametrize( + "router,type,nop_config,standard_config,docker_config,pyfunc_config,expected_class", [ + pytest.param( + "generic_router_config", + "nop", "nop_router_ensembler_config", None, None, None, + NopRouterEnsemblerConfig + ), + pytest.param( + "generic_router_config", + "standard", None, "standard_router_ensembler_config", None, None, + StandardRouterEnsemblerConfig + ), + pytest.param( + "generic_router_config", + "docker", None, None, "generic_ensembler_docker_config", None, + DockerRouterEnsemblerConfig + ), + pytest.param( + "generic_router_config", + "pyfunc", None, None, None, "generic_ensembler_pyfunc_config", + PyfuncRouterEnsemblerConfig + ) + ]) +def test_set_router_config_base_ensembler( + router, type, + nop_config, standard_config, docker_config, pyfunc_config, + expected_class, request): + actual = request.getfixturevalue(router) + ensembler = RouterEnsemblerConfig(type=type, + nop_config=None if nop_config is None else request.getfixturevalue(nop_config), + standard_config=None if standard_config is None else request.getfixturevalue(standard_config), + docker_config=None if docker_config is None else request.getfixturevalue(docker_config), + pyfunc_config=None if pyfunc_config is None else request.getfixturevalue(pyfunc_config) + ) + actual.ensembler = ensembler + assert isinstance(actual.ensembler, expected_class) + +@pytest.mark.parametrize( + "router_config,default_route_id,ensembler,expected", [ + pytest.param( + "generic_router_config", + "model-a", + StandardRouterEnsemblerConfig(experiment_mappings=[], fallback_response_route_id="model-b"), + "model-b", + ), + pytest.param( + "generic_router_config", + "model-a", + NopRouterEnsemblerConfig(final_response_route_id="model-b"), + "model-b", + ), + pytest.param( + "generic_router_config", + "model-a", + "docker_router_ensembler_config", + None, + ), + pytest.param( + "generic_router_config", + "model-a", + "pyfunc_router_ensembler_config", + None, + ) + ]) +def test_default_route_id_by_ensembler_config(router_config, default_route_id, ensembler, expected, request): + router = request.getfixturevalue(router_config) + router.default_route_id = default_route_id + router.ensembler = request.getfixturevalue(ensembler) if isinstance(ensembler, str) else ensembler + if expected: + assert router.to_open_api().to_dict()["config"]["default_route_id"] == expected + else: + assert "default_route_id" not in router.to_open_api().to_dict()["config"] diff --git a/sdk/tests/router/config/router_ensembler_config_test.py b/sdk/tests/router/config/router_ensembler_config_test.py index 47902f681..a7976c39e 100644 --- a/sdk/tests/router/config/router_ensembler_config_test.py +++ b/sdk/tests/router/config/router_ensembler_config_test.py @@ -6,7 +6,6 @@ from turing.router.config.route import InvalidRouteException from turing.router.config.router_ensembler_config import (RouterEnsemblerConfig, EnsemblerNopConfig, - EnsemblerStandardConfig, NopRouterEnsemblerConfig, PyfuncRouterEnsemblerConfig, DockerRouterEnsemblerConfig, @@ -18,21 +17,7 @@ pytest.param( 1, "standard", - EnsemblerStandardConfig( - experiment_mappings=[ - turing.generated.models.EnsemblerStandardConfigExperimentMappings( - experiment="experiment-1", - treatment="treatment-1", - route="route-1" - ), - turing.generated.models.EnsemblerStandardConfigExperimentMappings( - experiment="experiment-2", - treatment="treatment-2", - route="route-2" - ) - ], - fallback_response_route_id="route-1" - ), + "standard_router_ensembler_config", None, "generic_standard_router_ensembler_config" ), @@ -40,33 +25,18 @@ 1, "docker", None, - turing.generated.models.EnsemblerDockerConfig( - image="test.io/just-a-test/turing-ensembler:0.0.0-build.0", - resource_request=turing.generated.models.ResourceRequest( - min_replica=1, - max_replica=3, - cpu_request='100m', - memory_request='512Mi' - ), - endpoint=f"http://localhost:5000/ensembler_endpoint", - timeout="500ms", - port=5120, - env=[ - turing.generated.models.EnvVar( - name="env_name", - value="env_val") - ], - service_account="secret-name-for-google-service-account" - ), + "generic_ensembler_docker_config", "generic_docker_router_ensembler_config" ) ]) def test_create_router_ensembler_config(id, type, standard_config, docker_config, expected, request): + docker_config_data = None if docker_config is None else request.getfixturevalue(docker_config) + standard_config_data = None if standard_config is None else request.getfixturevalue(standard_config) actual = RouterEnsemblerConfig( id=id, type=type, - standard_config=standard_config, - docker_config=docker_config + standard_config=standard_config_data, + docker_config=docker_config_data ).to_open_api() assert actual == request.getfixturevalue(expected) @@ -532,3 +502,88 @@ def test_create_nop_router_ensembler_config_with_invalid_route( router.ensembler = ensembler_config with pytest.raises(expected): router.to_open_api() + +@pytest.mark.parametrize( + "cls,config,expected", [ + pytest.param( + NopRouterEnsemblerConfig, + "nop_router_ensembler_config", + {"type":"nop", "final_response_route_id": "test"} + ), + pytest.param( + StandardRouterEnsemblerConfig, + "standard_router_ensembler_config", + { + "type":"standard", + "experiment_mappings": [ + { + "experiment": "experiment-1", + "treatment": "treatment-1", + "route": "route-1" + }, + { + "experiment": "experiment-2", + "treatment": "treatment-2", + "route": "route-2" + } + ], + "fallback_response_route_id": "route-1" + } + ), + pytest.param( + DockerRouterEnsemblerConfig, + "generic_ensembler_docker_config", + { + "type": "docker", + "image": "test.io/just-a-test/turing-ensembler:0.0.0-build.0", + "resource_request": ResourceRequest( + min_replica=1, + max_replica=3, + cpu_request='100m', + memory_request='512Mi' + ), + "endpoint": "http://localhost:5000/ensembler_endpoint", + "timeout": "500ms", + "port": 5120, + "env": [EnvVar(name="env_name", value="env_val")], + "service_account": "secret-name-for-google-service-account" + } + ), + pytest.param( + PyfuncRouterEnsemblerConfig, + "generic_ensembler_pyfunc_config", + { + "type":"pyfunc", + "project_id": 77, + "ensembler_id": 11, + "resource_request": ResourceRequest( + min_replica=1, + max_replica=3, + cpu_request='100m', + memory_request='512Mi' + ), + "timeout": "500ms", + "env": [EnvVar(name="env_name", value="env_val")] + } + ) + ]) +def test_create_ensembler_config_from_config( + cls, config, expected, request +): + config_data = request.getfixturevalue(config) + assert cls.from_config(config_data).to_dict() == expected + +def test_set_nop_ensembler_config_with_default_route(request): + router = request.getfixturevalue("generic_router_config") + router.ensembler = None + router.default_route_id = "model-b" + assert router.ensembler.final_response_route_id == "model-b" + +def test_set_standard_ensembler_config_with_default_route(request): + router = request.getfixturevalue("generic_router_config") + router.ensembler = StandardRouterEnsemblerConfig( + experiment_mappings=[], + fallback_response_route_id="" + ) + router.default_route_id = "model-b" + assert router.ensembler.fallback_response_route_id == "model-b" diff --git a/sdk/turing/router/config/router_config.py b/sdk/turing/router/config/router_config.py index bb095841c..5e122c838 100644 --- a/sdk/turing/router/config/router_config.py +++ b/sdk/turing/router/config/router_config.py @@ -11,7 +11,13 @@ from turing.router.config.resource_request import ResourceRequest from turing.router.config.log_config import LogConfig, ResultLoggerType from turing.router.config.enricher import Enricher -from turing.router.config.router_ensembler_config import RouterEnsemblerConfig, NopRouterEnsemblerConfig, StandardRouterEnsemblerConfig +from turing.router.config.router_ensembler_config import ( + DockerRouterEnsemblerConfig, + NopRouterEnsemblerConfig, + PyfuncRouterEnsemblerConfig, + RouterEnsemblerConfig, + StandardRouterEnsemblerConfig, +) from turing.router.config.experiment_config import ExperimentConfig @@ -136,6 +142,13 @@ def default_route_id(self) -> str: details="Please use the ensembler properties to configure the final / fallback route.") def default_route_id(self, default_route_id: str): self._default_route_id = default_route_id + # User may directly modify the default_route_id property while it is deprecated. + # So, copy to the nop / standard ensembler if set. + if hasattr(self, "ensembler"): + if isinstance(self.ensembler, NopRouterEnsemblerConfig): + self.ensembler.final_response_route_id = default_route_id + elif isinstance(self.ensembler, StandardRouterEnsemblerConfig): + self.ensembler.fallback_response_route_id = default_route_id @property def experiment_engine(self) -> ExperimentConfig: @@ -206,8 +219,6 @@ def ensembler(self, ensembler: Union[RouterEnsemblerConfig, Dict]): if ensembler is None: # Init nop ensembler config if ensembler is not set self._ensembler = NopRouterEnsemblerConfig(final_response_route_id=self.default_route_id) - elif isinstance(ensembler, RouterEnsemblerConfig): - self._ensembler = ensembler elif isinstance(ensembler, dict): # Set fallback_response_route_id into standard ensembler config if ensembler["type"] == "standard" and "fallback_response_route_id" not in ensembler["standard_config"]: @@ -215,7 +226,17 @@ def ensembler(self, ensembler: Union[RouterEnsemblerConfig, Dict]): self._ensembler = RouterEnsemblerConfig(**ensembler) else: self._ensembler = ensembler - + # Init child class types + if isinstance(self._ensembler, RouterEnsemblerConfig): + if self._ensembler.type == "nop" and not isinstance(self._ensembler, NopRouterEnsemblerConfig): + self._ensembler = NopRouterEnsemblerConfig.from_config(self._ensembler.nop_config) + elif self._ensembler.type == "standard" and not isinstance(self._ensembler, StandardRouterEnsemblerConfig): + self._ensembler = StandardRouterEnsemblerConfig.from_config(self._ensembler.standard_config) + elif self._ensembler.type == "docker" and not isinstance(self._ensembler, DockerRouterEnsemblerConfig): + self._ensembler = DockerRouterEnsemblerConfig.from_config(self._ensembler.docker_config) + elif self._ensembler.type == "pyfunc" and not isinstance(self._ensembler, PyfuncRouterEnsemblerConfig): + self._ensembler = PyfuncRouterEnsemblerConfig.from_config(self._ensembler.pyfunc_config) + def to_open_api(self) -> OpenApiModel: kwargs = {} self._verify_no_duplicate_routes() @@ -250,7 +271,7 @@ def to_open_api(self) -> OpenApiModel: ) def _get_default_route_id(self): - default_route_id = self.default_route_id + default_route_id = None # If nop config is set, use the final_response_route_id as the default if isinstance(self.ensembler, NopRouterEnsemblerConfig): default_route_id = self.ensembler.final_response_route_id diff --git a/sdk/turing/router/config/router_ensembler_config.py b/sdk/turing/router/config/router_ensembler_config.py index 310cb6cea..bbe427f8b 100644 --- a/sdk/turing/router/config/router_ensembler_config.py +++ b/sdk/turing/router/config/router_ensembler_config.py @@ -1,5 +1,6 @@ from dataclasses import dataclass, field +from turing._base_types import DataObject import turing.generated.models from typing import List, Dict, Union from turing.generated.model_utils import OpenApiModel @@ -52,7 +53,7 @@ def to_open_api(self) -> OpenApiModel: @dataclass -class RouterEnsemblerConfig: +class RouterEnsemblerConfig(DataObject): """ Class to create a new RouterEnsemblerConfig @@ -251,6 +252,20 @@ def env(self) -> List['EnvVar']: def env(self, env: List['EnvVar']): self._env = env + @classmethod + def from_config(cls, config: turing.generated.models.EnsemblerPyfuncConfig) -> "PyfuncRouterEnsemblerConfig": + return cls( + project_id=config.project_id, + ensembler_id=config.ensembler_id, + timeout=config.timeout, + resource_request=ResourceRequest( + min_replica=config.resource_request.min_replica, + max_replica=config.resource_request.max_replica, + cpu_request=config.resource_request.cpu_request, + memory_request=config.resource_request.memory_request, + ), + env=[EnvVar(name=env.name, value=env.value) for env in config.env]) + def to_open_api(self) -> OpenApiModel: assert all(isinstance(env_var, EnvVar) for env_var in self.env) @@ -259,8 +274,7 @@ def to_open_api(self) -> OpenApiModel: ensembler_id=self.ensembler_id, resource_request=self.resource_request.to_open_api(), timeout=self.timeout, - env=[env_var.to_open_api() for env_var in self.env], - ) + env=[env_var.to_open_api() for env_var in self.env]) return super().to_open_api() @@ -350,6 +364,22 @@ def service_account(self) -> str: def service_account(self, service_account: str): self._service_account = service_account + @classmethod + def from_config(cls, config: turing.generated.models.EnsemblerDockerConfig) -> "DockerRouterEnsemblerConfig": + return cls( + image=config.image, + resource_request=ResourceRequest( + min_replica=config.resource_request.min_replica, + max_replica=config.resource_request.max_replica, + cpu_request=config.resource_request.cpu_request, + memory_request=config.resource_request.memory_request, + ), + endpoint=config.endpoint, + timeout=config.timeout, + port=config.port, + env=[EnvVar(name=env.name, value=env.value) for env in config.env], + service_account=config["service_account"]) + def to_open_api(self) -> OpenApiModel: assert all(isinstance(env_var, EnvVar) for env_var in self.env) @@ -413,6 +443,12 @@ def _verify_experiment_mappings(cls, experiment_mappings: List[Dict[str, str]]): f"experiment_mapping passed: {experiment_mapping}" ) + @classmethod + def from_config(cls, config: EnsemblerStandardConfig) -> "StandardRouterEnsemblerConfig": + return cls( + fallback_response_route_id=config.fallback_response_route_id, + experiment_mappings=[e.to_dict() for e in config.experiment_mappings]) + def to_open_api(self) -> OpenApiModel: self.standard_config = EnsemblerStandardConfig( experiment_mappings=[ @@ -441,6 +477,10 @@ def final_response_route_id(self) -> str: @final_response_route_id.setter def final_response_route_id(self, final_response_route_id: str): self._final_response_route_id = final_response_route_id + + @classmethod + def from_config(cls, config: EnsemblerNopConfig) -> "NopRouterEnsemblerConfig": + return cls(final_response_route_id=config.final_response_route_id) def to_open_api(self) -> OpenApiModel: self.nop_config = EnsemblerNopConfig(final_response_route_id=self.final_response_route_id) diff --git a/ui/src/services/ensembler/PyFuncEnsembler.js b/ui/src/services/ensembler/PyFuncEnsembler.js index 871f268ab..7d920ad88 100644 --- a/ui/src/services/ensembler/PyFuncEnsembler.js +++ b/ui/src/services/ensembler/PyFuncEnsembler.js @@ -13,6 +13,9 @@ export class PyFuncEnsembler extends Ensembler { static fromJson(json = {}) { const ensembler = new PyFuncEnsembler(); ensembler.pyfunc_config = objectAssignDeep({}, json.pyfunc_config); + if (!!json.docker_config) { + ensembler.docker_config = objectAssignDeep({}, json.docker_config); + } return ensembler; } diff --git a/ui/src/services/router/TuringRouter.js b/ui/src/services/router/TuringRouter.js index b180e30bf..bc301742f 100644 --- a/ui/src/services/router/TuringRouter.js +++ b/ui/src/services/router/TuringRouter.js @@ -109,19 +109,24 @@ export class TuringRouter { } // Ensembler - if (!!obj.config.ensembler) { - if (obj.config.ensembler.type === "nop") { - // Copy the final response route id to the top level, as the default route - obj.config.default_route_id = - obj.config["ensembler"].nop_config["final_response_route_id"]; - delete obj.config["ensembler"]; - } else if (obj.config.ensembler.type === "standard") { - // Copy the fallback response route id to the top level, as the default route - obj.config.default_route_id = - obj.config["ensembler"].standard_config["fallback_response_route_id"]; - delete obj.config["ensembler"].standard_config[ - "fallback_response_route_id" - ]; + if (obj.config.ensembler.type === "nop") { + // Copy the final response route id to the top level, as the default route + obj.config.default_route_id = + obj.config["ensembler"].nop_config["final_response_route_id"]; + delete obj.config["ensembler"]; + } else if (obj.config.ensembler.type === "standard") { + // Copy the fallback response route id to the top level, as the default route + obj.config.default_route_id = + obj.config["ensembler"].standard_config["fallback_response_route_id"]; + delete obj.config["ensembler"].standard_config[ + "fallback_response_route_id" + ]; + } else { + // Docker or Pyfunc ensembler, clear the default_route_id + delete obj.config["default_route_id"]; + if (obj.config.ensembler.type === "pyfunc") { + // Delete the docker config + delete obj.config["ensembler"].docker_config; } } @@ -138,7 +143,6 @@ export class TuringRouter { ) { delete obj.config.log_config["kafka_config"]; } - return obj; } }