From e845480578753d685f6f1f4554efa1d6f4c93d5f Mon Sep 17 00:00:00 2001 From: nadinet Date: Tue, 30 Mar 2021 14:08:30 +0100 Subject: [PATCH 01/10] Remove PERSISTENCE from python --- python/conda.recipe/meta.yaml | 1 - python/licenses/license_info.csv | 1 - python/licenses/license_info.no_versions.csv | 1 - python/seldon_core/microservice.py | 15 ++-- python/seldon_core/persistence.py | 86 -------------------- python/setup.py | 1 - 6 files changed, 10 insertions(+), 95 deletions(-) diff --git a/python/conda.recipe/meta.yaml b/python/conda.recipe/meta.yaml index c31be11d15..aa138c93a9 100644 --- a/python/conda.recipe/meta.yaml +++ b/python/conda.recipe/meta.yaml @@ -27,7 +27,6 @@ requirements: - protobuf - flask - flask-cors - - redis-py - tornado - requests - numpy diff --git a/python/licenses/license_info.csv b/python/licenses/license_info.csv index 10bc0fcc3b..eb12a8e05b 100644 --- a/python/licenses/license_info.csv +++ b/python/licenses/license_info.csv @@ -42,7 +42,6 @@ "pyasn1-modules","0.2.8","BSD License" "pycparser","2.20","BSD License" "pyrsistent","0.17.3","MIT License" -"redis","3.5.3","MIT License" "requests","2.25.1","Apache Software License" "requests-oauthlib","1.3.0","BSD License" "rsa","4.7.2","Apache Software License" diff --git a/python/licenses/license_info.no_versions.csv b/python/licenses/license_info.no_versions.csv index 8e98fffdef..dab5e782f4 100644 --- a/python/licenses/license_info.no_versions.csv +++ b/python/licenses/license_info.no_versions.csv @@ -42,7 +42,6 @@ "pyasn1-modules","BSD License" "pycparser","BSD License" "pyrsistent","MIT License" -"redis","MIT License" "requests","Apache Software License" "requests-oauthlib","BSD License" "rsa","Apache Software License" diff --git a/python/seldon_core/microservice.py b/python/seldon_core/microservice.py index 0aba339796..4b7b16dea4 100644 --- a/python/seldon_core/microservice.py +++ b/python/seldon_core/microservice.py @@ -10,7 +10,7 @@ from functools import partial from typing import Callable, Dict -from seldon_core import __version__, persistence +from seldon_core import __version__ from seldon_core import wrapper as seldon_microservice from seldon_core.flask_utils import ANNOTATIONS_FILE, SeldonMicroserviceException from seldon_core.gunicorn_utils import ( @@ -213,7 +213,14 @@ def main(): choices=["MODEL", "ROUTER", "TRANSFORMER", "COMBINER", "OUTLIER_DETECTOR"], default="MODEL", ) - parser.add_argument("--persistence", nargs="?", default=0, const=1, type=int) + parser.add_argument( + "--persistence", + nargs="?", + default=0, + const=1, + type=int, + help="deprecated argument ", + ) parser.add_argument( "--parameters", type=str, default=os.environ.get(PARAMETERS_ENV_NAME, "[]") ) @@ -345,9 +352,7 @@ def main(): user_class = getattr(interface_file, parts[1]) if args.persistence: - logger.info("Restoring persisted component") - user_object = persistence.restore(user_class, parameters) - persistence.persist(user_object, parameters.get("push_frequency")) + logger.error(f"persistence is not supported") else: user_object = user_class(**parameters) diff --git a/python/seldon_core/persistence.py b/python/seldon_core/persistence.py index 45010dd3fa..e69de29bb2 100644 --- a/python/seldon_core/persistence.py +++ b/python/seldon_core/persistence.py @@ -1,86 +0,0 @@ -import logging -import os -import pickle -import threading -import time -from typing import Dict, Type - -import redis - -from seldon_core.user_model import SeldonComponent - -logger = logging.getLogger(__name__) - -PRED_UNIT_ID = os.environ.get("PREDICTIVE_UNIT_ID", "0") -PREDICTOR_ID = os.environ.get("PREDICTOR_ID", "0") -DEPLOYMENT_ID = os.environ.get("SELDON_DEPLOYMENT_ID", "0") -REDIS_KEY = f"persistence_{DEPLOYMENT_ID}_{PREDICTOR_ID}_{PRED_UNIT_ID}" -REDIS_HOST = os.environ.get("REDIS_SERVICE_HOST", "localhost") -REDIS_PORT = os.environ.get("REDIS_SERVICE_PORT", 6379) -DEFAULT_PUSH_FREQUENCY = 60 - - -def restore(user_class: Type[SeldonComponent], parameters: Dict) -> SeldonComponent: - """ - Restore saved state from Redis - - Parameters - ---------- - user_class - User class - parameters - The parameters for the class - - Returns - ------- - A restored class or a new one - - """ - logger.info("Restoring saved model from redis") - - redis_client = redis.StrictRedis(host=REDIS_HOST, port=REDIS_PORT) - saved_state_binary = redis_client.get(REDIS_KEY) - if saved_state_binary is None: - logger.info("Saved state is empty, restoration aborted") - return user_class(**parameters) - else: - return pickle.loads(saved_state_binary) - - -def persist(user_object: SeldonComponent, push_frequency: int = None): - """ - Start a thread to persist a user class to Redis - - Parameters - ---------- - user_object - A user class object - push_frequency - How often to save state (secs) - - """ - - if push_frequency is None: - push_frequency = DEFAULT_PUSH_FREQUENCY - logger.info("Creating persistence thread, with frequency %s", push_frequency) - persistence_thread = PersistenceThread(user_object, push_frequency) - persistence_thread.start() - - -class PersistenceThread(threading.Thread): - def __init__(self, user_object, push_frequency): - self.user_object = user_object - self.push_frequency = push_frequency - self._stopped = False - self.redis_client = redis.StrictRedis(host=REDIS_HOST, port=REDIS_PORT) - super(PersistenceThread, self).__init__() - - def stop(self): - logger.info("Stopping Persistence Thread") - self._stopped = True - - def run(self): - while not self._stopped: - time.sleep(self.push_frequency) - binary_data = pickle.dumps(self.user_object) - self.redis_client.set(REDIS_KEY, binary_data) diff --git a/python/setup.py b/python/setup.py index 3ad2e99e96..edd782959e 100644 --- a/python/setup.py +++ b/python/setup.py @@ -24,7 +24,6 @@ "Flask<2.0.0", "jsonschema<4.0.0", "Flask-cors<4.0.0", - "redis<4.0.0", "requests<3.0.0", "numpy<2.0.0", "flatbuffers<2.0.0", From a758d817e7189912c8835ebef2e961944ca15e33 Mon Sep 17 00:00:00 2001 From: nadinet Date: Tue, 30 Mar 2021 14:50:00 +0100 Subject: [PATCH 02/10] deleate persistence.py --- python/seldon_core/persistence.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 python/seldon_core/persistence.py diff --git a/python/seldon_core/persistence.py b/python/seldon_core/persistence.py deleted file mode 100644 index e69de29bb2..0000000000 From 0c65d92a0c1959404798de5d06d8f0202ea50a02 Mon Sep 17 00:00:00 2001 From: nadinet Date: Wed, 31 Mar 2021 11:37:34 +0100 Subject: [PATCH 03/10] Remove PERSISTENCE from python: Unit testing --- python/seldon_core/microservice.py | 5 ++--- python/tests/helpers.py | 20 ++++++++++---------- python/tests/test_microservice.py | 9 +++++++++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/python/seldon_core/microservice.py b/python/seldon_core/microservice.py index 4b7b16dea4..4af2c01cff 100644 --- a/python/seldon_core/microservice.py +++ b/python/seldon_core/microservice.py @@ -352,9 +352,8 @@ def main(): user_class = getattr(interface_file, parts[1]) if args.persistence: - logger.error(f"persistence is not supported") - else: - user_object = user_class(**parameters) + logger.error(f"persistence: ignored, persistence is deprecated") + user_object = user_class(**parameters) http_port = args.http_port grpc_port = args.grpc_port diff --git a/python/tests/helpers.py b/python/tests/helpers.py index d606def77e..cc9ee6eb73 100644 --- a/python/tests/helpers.py +++ b/python/tests/helpers.py @@ -23,6 +23,16 @@ def __init__(self, app_location, envs={}, tracing=False): def _env_vars(self, envs): env_vars = dict(os.environ) + s2i_env_file = os.path.join(self.app_location, ".s2i", "environment") + with open(s2i_env_file) as fh: + for line in fh.readlines(): + line = line.strip() + if line: + key, value = line.split("=", 1) + key, value = key.strip(), value.strip() + if key and value: + env_vars[key] = value + env_vars.update(envs) env_vars.update( { @@ -36,16 +46,6 @@ def _env_vars(self, envs): } ) - s2i_env_file = os.path.join(self.app_location, ".s2i", "environment") - with open(s2i_env_file) as fh: - for line in fh.readlines(): - line = line.strip() - if line: - key, value = line.split("=", 1) - key, value = key.strip(), value.strip() - if key and value: - env_vars[key] = value - return env_vars def _get_cmd(self, tracing): diff --git a/python/tests/test_microservice.py b/python/tests/test_microservice.py index bf415fff5f..4b5b061e5c 100644 --- a/python/tests/test_microservice.py +++ b/python/tests/test_microservice.py @@ -249,3 +249,12 @@ def test_load_annotations(mock_isfile): def test_health_status(microservice): response = requests.get("http://127.0.0.1:9000/health/status") response.raise_for_status() + + +@pytest.mark.parametrize( + "microservice", [{"envs": {"PERSISTENCE": "1"}}], indirect=True +) +def test_persistence_flag_exist(microservice): + response = requests.get("http://127.0.0.1:9000/health/status") + # This just tests that persistence flag was not removed ("soft" deprecation) + response.raise_for_status() From b642c868ae784b1e6cd93a5d4b531678c9053c77 Mon Sep 17 00:00:00 2001 From: nadinet Date: Wed, 31 Mar 2021 12:27:48 +0100 Subject: [PATCH 04/10] Persistence remove: removed PERSISTENCE from s2i environment variables --- python/tests/helpers.py | 4 ++-- python/tests/resources/custom-metrics-model/.s2i/environment | 1 - python/tests/resources/model-template-app/.s2i/environment | 1 - python/tests/resources/model-template-app2/.s2i/environment | 1 - 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/python/tests/helpers.py b/python/tests/helpers.py index cc9ee6eb73..fe19a87c20 100644 --- a/python/tests/helpers.py +++ b/python/tests/helpers.py @@ -54,9 +54,9 @@ def _get_cmd(self, tracing): self.env_vars["MODEL_NAME"], "--service-type", self.env_vars["SERVICE_TYPE"], - "--persistence", - self.env_vars["PERSISTENCE"], ) + if "PERSISTENCE" in self.env_vars: + cmd += ("--persistence", self.env_vars["PERSISTENCE"]) if tracing: cmd += ("--tracing",) diff --git a/python/tests/resources/custom-metrics-model/.s2i/environment b/python/tests/resources/custom-metrics-model/.s2i/environment index dac45a6595..d4b6cc3bff 100644 --- a/python/tests/resources/custom-metrics-model/.s2i/environment +++ b/python/tests/resources/custom-metrics-model/.s2i/environment @@ -1,4 +1,3 @@ MODEL_NAME=MyModel API_TYPE=REST SERVICE_TYPE=MODEL -PERSISTENCE=0 diff --git a/python/tests/resources/model-template-app/.s2i/environment b/python/tests/resources/model-template-app/.s2i/environment index dac45a6595..d4b6cc3bff 100644 --- a/python/tests/resources/model-template-app/.s2i/environment +++ b/python/tests/resources/model-template-app/.s2i/environment @@ -1,4 +1,3 @@ MODEL_NAME=MyModel API_TYPE=REST SERVICE_TYPE=MODEL -PERSISTENCE=0 diff --git a/python/tests/resources/model-template-app2/.s2i/environment b/python/tests/resources/model-template-app2/.s2i/environment index a2cb7cf804..592e093360 100644 --- a/python/tests/resources/model-template-app2/.s2i/environment +++ b/python/tests/resources/model-template-app2/.s2i/environment @@ -1,4 +1,3 @@ MODEL_NAME=mymodule.my_model.MyModel API_TYPE=REST SERVICE_TYPE=MODEL -PERSISTENCE=0 From 5b13d4506f867c503bc37c227f4007b4d2965422 Mon Sep 17 00:00:00 2001 From: nadinet Date: Wed, 31 Mar 2021 14:32:08 +0100 Subject: [PATCH 05/10] Remove Persistence from documentation --- doc/source/python/python_wrapping_docker.md | 6 +----- doc/source/python/python_wrapping_s2i.md | 4 ---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/doc/source/python/python_wrapping_docker.md b/doc/source/python/python_wrapping_docker.md index 22b307d5d6..31993ba5e1 100644 --- a/doc/source/python/python_wrapping_docker.md +++ b/doc/source/python/python_wrapping_docker.md @@ -66,9 +66,8 @@ EXPOSE 5000 # Define environment variable ENV MODEL_NAME MyModel ENV SERVICE_TYPE MODEL -ENV PERSISTENCE 0 -CMD exec seldon-core-microservice $MODEL_NAME --service-type $SERVICE_TYPE --persistence $PERSISTENCE +CMD exec seldon-core-microservice $MODEL_NAME --service-type $SERVICE_TYPE ``` @@ -96,9 +95,6 @@ The service type being created. Available options are: * COMBINER * OUTLIER_DETECTOR -### PERSISTENCE - -Set either to 0 or 1. Default is 0. If set to 1 then your model will be saved periodically to redis and loaded from redis (if exists) or created fresh if not. ### Flask Settings diff --git a/doc/source/python/python_wrapping_s2i.md b/doc/source/python/python_wrapping_s2i.md index 5a7ac72786..c20146eafb 100644 --- a/doc/source/python/python_wrapping_s2i.md +++ b/doc/source/python/python_wrapping_s2i.md @@ -89,7 +89,6 @@ Define the core parameters needed by our python builder image to wrap your model ```bash MODEL_NAME=MyModel SERVICE_TYPE=MODEL -PERSISTENCE=0 ``` These values can also be provided or overridden on the command line when building the image. @@ -161,9 +160,6 @@ The service type being created. Available options are: * COMBINER * OUTLIER_DETECTOR -### PERSISTENCE - -Set either to 0 or 1. Default is 0. If set to 1 then your model will be saved periodically to redis and loaded from redis (if exists) or created fresh if not. ### EXTRA_INDEX_URL From 5a9b9dc4e8e92a0401b78ce6fb81dda921414f5e Mon Sep 17 00:00:00 2001 From: nadinet Date: Wed, 31 Mar 2021 16:05:37 +0100 Subject: [PATCH 06/10] Fix typo --- python/seldon_core/microservice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/seldon_core/microservice.py b/python/seldon_core/microservice.py index 4af2c01cff..b4f04a6050 100644 --- a/python/seldon_core/microservice.py +++ b/python/seldon_core/microservice.py @@ -352,7 +352,7 @@ def main(): user_class = getattr(interface_file, parts[1]) if args.persistence: - logger.error(f"persistence: ignored, persistence is deprecated") + logger.error(f"Persistence: ignored, persistence is deprecated") user_object = user_class(**parameters) http_port = args.http_port From f50b1a8e835842503c7117896d9f49b084d0a012 Mon Sep 17 00:00:00 2001 From: nadinet Date: Thu, 1 Apr 2021 18:39:13 +0100 Subject: [PATCH 07/10] remove persistence reference from s2i run script --- wrappers/s2i/python/s2i/bin/run | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/wrappers/s2i/python/s2i/bin/run b/wrappers/s2i/python/s2i/bin/run index c7b3400509..5bab4e2dc4 100755 --- a/wrappers/s2i/python/s2i/bin/run +++ b/wrappers/s2i/python/s2i/bin/run @@ -22,9 +22,9 @@ function _is_env_present() { } #check environment vars -if [[ -z "$MODEL_NAME" || -z "$SERVICE_TYPE" || -z "$PERSISTENCE" ]]; then +if [[ -z "$MODEL_NAME" || -z "$SERVICE_TYPE" ]]; then - echo "Failed to find required env vars MODEL_NAME, SERVICE_TYPE, PERSISTENCE" + echo "Failed to find required env vars MODEL_NAME, SERVICE_TYPE" exit 1 else @@ -44,6 +44,9 @@ else fi echo "starting microservice" - exec seldon-core-microservice $MODEL_NAME --service-type $SERVICE_TYPE --persistence $PERSISTENCE - + if[-n "$PERSISTENCE"]; then + exec seldon-core-microservice $MODEL_NAME --service-type $SERVICE_TYPE --persistence $PERSISTENCE + else + exec seldon-core-microservice $MODEL_NAME --service-type $SERVICE_TYPE + fi fi From e2502b8d0c52a1eed42c30e7937203373e74be9b Mon Sep 17 00:00:00 2001 From: Alejandro Saucedo Date: Wed, 7 Apr 2021 21:32:16 +0100 Subject: [PATCH 08/10] Updated syntax error --- wrappers/s2i/python/s2i/bin/run | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wrappers/s2i/python/s2i/bin/run b/wrappers/s2i/python/s2i/bin/run index 5bab4e2dc4..dfe66ec843 100755 --- a/wrappers/s2i/python/s2i/bin/run +++ b/wrappers/s2i/python/s2i/bin/run @@ -44,7 +44,7 @@ else fi echo "starting microservice" - if[-n "$PERSISTENCE"]; then + if [ -n "$PERSISTENCE" ]; then exec seldon-core-microservice $MODEL_NAME --service-type $SERVICE_TYPE --persistence $PERSISTENCE else exec seldon-core-microservice $MODEL_NAME --service-type $SERVICE_TYPE From d098d82484c938020efcd4d37519e202c41dfe4a Mon Sep 17 00:00:00 2001 From: Alejandro Saucedo Date: Wed, 7 Apr 2021 22:29:30 +0100 Subject: [PATCH 09/10] Removed explicit dependency installation given the dependency does carry over --- servers/mlflowserver/mlflowserver/requirements.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/servers/mlflowserver/mlflowserver/requirements.txt b/servers/mlflowserver/mlflowserver/requirements.txt index 547fca5838..3bf1ebc743 100644 --- a/servers/mlflowserver/mlflowserver/requirements.txt +++ b/servers/mlflowserver/mlflowserver/requirements.txt @@ -2,6 +2,3 @@ pyyaml==5.4.0 requests<2.25.0 mlflow<1.12.0 pandas<1.2.0 - -# local seldon-core inside the s2i image -/microservice/python From 2e485ec483235af22cd8dff00ae03c84f52d21e3 Mon Sep 17 00:00:00 2001 From: Alejandro Saucedo Date: Thu, 8 Apr 2021 13:40:58 +0100 Subject: [PATCH 10/10] Reverting --- servers/mlflowserver/mlflowserver/requirements.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/servers/mlflowserver/mlflowserver/requirements.txt b/servers/mlflowserver/mlflowserver/requirements.txt index 3bf1ebc743..547fca5838 100644 --- a/servers/mlflowserver/mlflowserver/requirements.txt +++ b/servers/mlflowserver/mlflowserver/requirements.txt @@ -2,3 +2,6 @@ pyyaml==5.4.0 requests<2.25.0 mlflow<1.12.0 pandas<1.2.0 + +# local seldon-core inside the s2i image +/microservice/python