diff --git a/components/alibi-detect-server/adserver/cm_model.py b/components/alibi-detect-server/adserver/cm_model.py index e83ec9a29e..e2482c7b29 100644 --- a/components/alibi-detect-server/adserver/cm_model.py +++ b/components/alibi-detect-server/adserver/cm_model.py @@ -11,7 +11,8 @@ from adserver.base import CEModel, ModelResponse from adserver.base.storage import download_model from seldon_core.flask_utils import SeldonMicroserviceException -from seldon_core.metrics import DEFAULT_LABELS, NONIMPLEMENTED_MSG +from seldon_core.metrics import DEFAULT_LABELS +from seldon_core.env_utils import NONIMPLEMENTED_MSG from elasticsearch import Elasticsearch from elasticsearch.exceptions import NotFoundError diff --git a/python/requirements-dev.txt b/python/requirements-dev.txt index 36868319de..ef3749a658 100644 --- a/python/requirements-dev.txt +++ b/python/requirements-dev.txt @@ -11,5 +11,7 @@ pillow==8.1.1 pip-licenses==3.4.0 # fetch licenses pytest-cov==2.10.1 pytest==6.2.2 +pytest-mock==3.6.1 tenacity==6.3.1 # tenacity - used for smart retrying +nbqa==0.13.1 tox<4.0.0 diff --git a/python/seldon_core/env_utils.py b/python/seldon_core/env_utils.py new file mode 100644 index 0000000000..4107b96467 --- /dev/null +++ b/python/seldon_core/env_utils.py @@ -0,0 +1,110 @@ +""" +Utilities to deal with Environment variables +""" +import json +import os + +ENV_MODEL_NAME = "PREDICTIVE_UNIT_ID" +ENV_MODEL_IMAGE = "PREDICTIVE_UNIT_IMAGE" +ENV_SELDON_DEPLOYMENT_NAME = "SELDON_DEPLOYMENT_ID" +ENV_PREDICTOR_NAME = "PREDICTOR_ID" +ENV_PREDICTOR_LABELS = "PREDICTOR_LABELS" +NONIMPLEMENTED_MSG = "NOT_IMPLEMENTED" +NONIMPLEMENTED_IMAGE_MSG = f"{NONIMPLEMENTED_MSG}:{NONIMPLEMENTED_MSG}" + + +def get_predictor_version(default_val: str = NONIMPLEMENTED_MSG) -> str: + """ + Get predictor version from `ENV_PREDICTOR_LABELS` environment variable. + If not set return `default_val` + + + Parameters + ---------- + default_val + Default value to return if the environment variable is not set + + Returns + ------- + str + + """ + return json.loads(os.environ.get(ENV_PREDICTOR_LABELS, "{}")).get( + "version", default_val + ) + + +def get_predictor_name(default_val: str = NONIMPLEMENTED_MSG) -> str: + """ + Get predictor name from `ENV_PREDICTOR_NAME` environment variable. + If not set return `default_val` + + + Parameters + ---------- + default_val + Default value to return if the environment variable is not set + + Returns + ------- + str + + """ + return os.environ.get(ENV_PREDICTOR_NAME, default_val) + + +def get_deployment_name(default_val: str = NONIMPLEMENTED_MSG) -> str: + """ + Get deployment name from `ENV_SELDON_DEPLOYMENT_NAME` environment variable. + If not set return `default_val` + + + Parameters + ---------- + default_val + Default value to return if the environment variable is not set + + Returns + ------- + str + + """ + return os.environ.get(ENV_SELDON_DEPLOYMENT_NAME, default_val) + + +def get_model_name(default_val: str = NONIMPLEMENTED_MSG) -> str: + """ + Get model name from `ENV_MODEL_NAME` environment variable. + If not set return `default_val` + + + Parameters + ---------- + default_val + Default value to return if the environment variable is not set + + Returns + ------- + str + + """ + return os.environ.get(ENV_MODEL_NAME, default_val) + + +def get_image_name(default_val: str = NONIMPLEMENTED_IMAGE_MSG) -> str: + """ + Get model image name from `ENV_MODEL_IMAGE` environment variable. + If not set return `default_val` + + + Parameters + ---------- + default_val + Default value to return if the environment variable is not set + + Returns + ------- + str + + """ + return os.environ.get(ENV_MODEL_IMAGE, default_val) diff --git a/python/seldon_core/metrics.py b/python/seldon_core/metrics.py index c01efca89c..15fe3a26b7 100644 --- a/python/seldon_core/metrics.py +++ b/python/seldon_core/metrics.py @@ -1,4 +1,3 @@ -import json import logging import os from multiprocessing import Manager @@ -14,15 +13,15 @@ ) from prometheus_client.utils import floatToGoString -logger = logging.getLogger(__name__) - -NONIMPLEMENTED_MSG = "NOT_IMPLEMENTED" +from seldon_core.env_utils import ( + get_deployment_name, + get_image_name, + get_model_name, + get_predictor_name, + get_predictor_version, +) -ENV_SELDON_DEPLOYMENT_NAME = "SELDON_DEPLOYMENT_ID" -ENV_MODEL_NAME = "PREDICTIVE_UNIT_ID" -ENV_MODEL_IMAGE = "PREDICTIVE_UNIT_IMAGE" -ENV_PREDICTOR_NAME = "PREDICTOR_ID" -ENV_PREDICTOR_LABELS = "PREDICTOR_LABELS" +logger = logging.getLogger(__name__) FEEDBACK_KEY = "seldon_api_model_feedback" FEEDBACK_REWARD_KEY = "seldon_api_model_feedback_reward" @@ -53,22 +52,18 @@ def split_image_tag(tag: str) -> Tuple[str]: # Development placeholder -image = os.environ.get(ENV_MODEL_IMAGE, f"{NONIMPLEMENTED_MSG}:{NONIMPLEMENTED_MSG}") +image = get_image_name() model_image, model_version = split_image_tag(image) -predictor_version = json.loads(os.environ.get(ENV_PREDICTOR_LABELS, "{}")).get( - "version", f"{NONIMPLEMENTED_MSG}" -) +predictor_version = get_predictor_version() legacy_mode = os.environ.get("SELDON_EXECUTOR_ENABLED", "true").lower() == "false" DEFAULT_LABELS = { - "deployment_name": os.environ.get( - ENV_SELDON_DEPLOYMENT_NAME, f"{NONIMPLEMENTED_MSG}" - ), - "model_name": os.environ.get(ENV_MODEL_NAME, f"{NONIMPLEMENTED_MSG}"), + "deployment_name": get_deployment_name(), + "model_name": get_model_name(), "model_image": model_image, "model_version": model_version, - "predictor_name": os.environ.get(ENV_PREDICTOR_NAME, f"{NONIMPLEMENTED_MSG}"), + "predictor_name": get_predictor_name(), "predictor_version": predictor_version, } diff --git a/python/seldon_core/utils.py b/python/seldon_core/utils.py index 6a1ef463b6..b1d4fe0eb2 100644 --- a/python/seldon_core/utils.py +++ b/python/seldon_core/utils.py @@ -10,6 +10,14 @@ from google.protobuf.json_format import MessageToDict, ParseDict from google.protobuf.struct_pb2 import ListValue +from seldon_core.env_utils import ( + ENV_MODEL_IMAGE, + ENV_MODEL_NAME, + NONIMPLEMENTED_IMAGE_MSG, + NONIMPLEMENTED_MSG, + get_image_name, + get_model_name, +) from seldon_core.flask_utils import SeldonMicroserviceException from seldon_core.imports_helper import _TF_PRESENT from seldon_core.proto import prediction_pb2 @@ -27,19 +35,14 @@ logger = logging.getLogger(__name__) -ENV_MODEL_NAME = "PREDICTIVE_UNIT_ID" -ENV_MODEL_IMAGE = "PREDICTIVE_UNIT_IMAGE" -NONIMPLEMENTED_MSG = "NOT_IMPLEMENTED" - -model_name = os.environ.get(ENV_MODEL_NAME, f"{NONIMPLEMENTED_MSG}") -image_name = os.environ.get( - ENV_MODEL_IMAGE, f"{NONIMPLEMENTED_MSG}:{NONIMPLEMENTED_MSG}" -) - - def get_request_path(): + model_name = get_model_name() if model_name == NONIMPLEMENTED_MSG: return {} + image_name = get_image_name() + assert ( + image_name != NONIMPLEMENTED_IMAGE_MSG + ), f"Both {ENV_MODEL_NAME} and {ENV_MODEL_IMAGE} have to be set" return {model_name: image_name} @@ -164,6 +167,7 @@ def feedback_to_json(message_proto: prediction_pb2.Feedback) -> Dict: ---------- message_proto SeldonMessage proto + Returns ------- JSON Dict @@ -348,6 +352,7 @@ def array_to_list_value(array: np.ndarray, lv: Optional[ListValue] = None) -> Li Returns ------- + ListValue protobuf """ if lv is None: diff --git a/python/seldon_core/wrapper.py b/python/seldon_core/wrapper.py index 113e059554..61a497ccd0 100644 --- a/python/seldon_core/wrapper.py +++ b/python/seldon_core/wrapper.py @@ -8,6 +8,7 @@ from grpc_reflection.v1alpha import reflection import seldon_core.seldon_methods +from seldon_core.env_utils import get_model_name from seldon_core.flask_utils import ( ANNOTATION_GRPC_MAX_MSG_SIZE, SeldonMicroserviceException, @@ -24,7 +25,7 @@ logger = logging.getLogger(__name__) -PRED_UNIT_ID = os.environ.get("PREDICTIVE_UNIT_ID", "0") +PRED_UNIT_ID = get_model_name("0") METRICS_ENDPOINT = os.environ.get("PREDICTIVE_UNIT_METRICS_ENDPOINT", "/metrics") PAYLOAD_PASSTHROUGH = getenv_as_bool("PAYLOAD_PASSTHROUGH", default=False) diff --git a/python/tests/test_model_microservice.py b/python/tests/test_model_microservice.py index cf446e289c..23c6453845 100644 --- a/python/tests/test_model_microservice.py +++ b/python/tests/test_model_microservice.py @@ -2,9 +2,9 @@ import io import json import logging -from unittest import mock import numpy as np +import pytest from google.protobuf import json_format from PIL import Image @@ -240,6 +240,22 @@ def send_feedback_grpc(self, request): logging.info("Feedback called") +@pytest.fixture(name="mock_get_model_name") +def fixture_get_model_name(mocker): + return mocker.patch( + "seldon_core.utils.get_model_name", autospec=True, return_value="my-test-model" + ) + + +@pytest.fixture(name="mock_get_image_name") +def fixture_get_image_name(mocker): + return mocker.patch( + "seldon_core.utils.get_image_name", + autospec=True, + return_value="my-test-model-image", + ) + + def test_model_ok(): user_object = UserObject() seldon_metrics = SeldonMetrics() @@ -311,9 +327,7 @@ def test_model_puid_ok(): assert j["meta"]["puid"] == "123" -@mock.patch("seldon_core.utils.model_name", "my-test-model") -@mock.patch("seldon_core.utils.image_name", "my-test-model-image") -def test_requestPath_ok(): +def test_requestPath_ok(mock_get_model_name, mock_get_image_name): user_object = UserObject() seldon_metrics = SeldonMetrics() app = get_rest_microservice(user_object, seldon_metrics) @@ -327,9 +341,7 @@ def test_requestPath_ok(): assert j["meta"]["requestPath"] == {"my-test-model": "my-test-model-image"} -@mock.patch("seldon_core.utils.model_name", "my-test-model") -@mock.patch("seldon_core.utils.image_name", "my-test-model-image") -def test_requestPath_2nd_node_ok(): +def test_requestPath_2nd_node_ok(mock_get_model_name, mock_get_image_name): user_object = UserObject() seldon_metrics = SeldonMetrics() app = get_rest_microservice(user_object, seldon_metrics) @@ -346,9 +358,7 @@ def test_requestPath_2nd_node_ok(): } -@mock.patch("seldon_core.utils.model_name", "my-test-model") -@mock.patch("seldon_core.utils.image_name", "my-test-model-image") -def test_proto_requestPath_ok(): +def test_proto_requestPath_ok(mock_get_model_name, mock_get_image_name): user_object = UserObject() seldon_metrics = SeldonMetrics() app = SeldonModelGRPC(user_object, seldon_metrics) @@ -366,9 +376,7 @@ def test_proto_requestPath_ok(): assert j["meta"]["requestPath"] == {"my-test-model": "my-test-model-image"} -@mock.patch("seldon_core.utils.model_name", "my-test-model") -@mock.patch("seldon_core.utils.image_name", "my-test-model-image") -def test_proto_requestPath_2nd_node_ok(): +def test_proto_requestPath_2nd_node_ok(mock_get_model_name, mock_get_image_name): user_object = UserObject() seldon_metrics = SeldonMetrics() app = SeldonModelGRPC(user_object, seldon_metrics) diff --git a/python/tests/test_utils.py b/python/tests/test_utils.py index 5facc2a445..2099456ed1 100644 --- a/python/tests/test_utils.py +++ b/python/tests/test_utils.py @@ -1,4 +1,5 @@ import base64 +import json import logging import pickle @@ -8,6 +9,20 @@ from google.protobuf.struct_pb2 import Value import seldon_core.utils as scu +from seldon_core.env_utils import ( + ENV_MODEL_IMAGE, + ENV_MODEL_NAME, + ENV_PREDICTOR_LABELS, + ENV_PREDICTOR_NAME, + ENV_SELDON_DEPLOYMENT_NAME, + NONIMPLEMENTED_IMAGE_MSG, + NONIMPLEMENTED_MSG, + get_deployment_name, + get_image_name, + get_model_name, + get_predictor_name, + get_predictor_version, +) from seldon_core.flask_utils import SeldonMicroserviceException from seldon_core.imports_helper import _TF_PRESENT from seldon_core.proto import prediction_pb2 @@ -469,3 +484,67 @@ def test_getenv_as_bool(monkeypatch, env_val, expected): value = scu.getenv_as_bool(env_var, default=False) assert value == expected + + +class TestEnvironmentVariables: + """ + Tests for getting values from environment variables + """ + + @pytest.mark.parametrize( + "val, expected_val, env_var, getter", + [ + ( + "DUMMY_VAL_NAME", + "DUMMY_VAL_NAME", + ENV_SELDON_DEPLOYMENT_NAME, + get_deployment_name, + ), + ("DUMMY_VAL_NAME", "DUMMY_VAL_NAME", ENV_MODEL_NAME, get_model_name), + ("DUMMY_VAL_NAME", "DUMMY_VAL_NAME", ENV_MODEL_IMAGE, get_image_name), + ( + "DUMMY_VAL_NAME", + "DUMMY_VAL_NAME", + ENV_PREDICTOR_NAME, + get_predictor_name, + ), + ( + json.dumps({"key": "dummy", "version": "2"}), + "2", + ENV_PREDICTOR_LABELS, + get_predictor_version, + ), + ], + ) + def test_get_deployment_name_ok( + self, monkeypatch, val, expected_val, env_var, getter + ): + monkeypatch.setenv(env_var, val) + assert getter() == expected_val + + @pytest.mark.parametrize( + "val, getter", + [ + (NONIMPLEMENTED_MSG, get_deployment_name), + (NONIMPLEMENTED_MSG, get_model_name), + (NONIMPLEMENTED_IMAGE_MSG, get_image_name), + (NONIMPLEMENTED_MSG, get_predictor_name), + (NONIMPLEMENTED_MSG, get_predictor_version), + ], + ) + def test_env_notset_ok(self, val, getter): + assert getter() == val + + @pytest.mark.parametrize( + "val, getter", + [ + ("0", get_deployment_name), + ("0", get_model_name), + ("0", get_model_name), + ("0", get_image_name), + ("0", get_predictor_name), + ("0", get_predictor_version), + ], + ) + def test_env_notset_with_default_ok(self, val, getter): + assert getter(default_val=val) == val