Merge pull request #488 from cliveseldon/0.2.7_release

Updates for various Python and Operator fixes

.gitignore

@@ -192,7 +192,10 @@ doc/source/_static/cluster-manager
 python/build/
 python/dist/
 testing/scripts/proto
+
+python/.coverage
+
 testing/scripts/tensorflow
 testing/scripts/run.log
 testing/scripts/my-model/
-wrappers/s2i/python/_python/
+wrappers/s2i/python/_python/

api-frontend/src/main/java/io/seldon/apife/deployments/DeploymentStore.java

@@ -60,7 +60,10 @@ private void init() throws Exception{
 
 	public SeldonDeployment getDeployment(String clientId)
 	{
-		return deploymentStore.get(clientId);
+		if (clientId != null)
+			return deploymentStore.get(clientId);
+		else
+			return null;
 	}
 
 	 @Override

cluster-manager/src/main/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateImpl.java

@@ -105,7 +105,7 @@ public void updateStatus(String mlDepName, String version, String depName, Integ
         {
             logger.info(String.format("UPDATE %s : %s %d %d",mlDepName,depName,replicas,replicasAvailable));
             SeldonDeployment mlDep = crdHandler.getSeldonDeployment(mlDepName,version, namespace);
-            if (mlDep != null)
+            if (mlDep != null && !(mlDep.getStatus().hasState() && mlDep.getStatus().getState().equals(Constants.STATE_FAILED)))
             {
                 SeldonDeployment.Builder mlBuilder = SeldonDeployment.newBuilder(mlDep);
 

cluster-manager/src/test/java/io/seldon/clustermanager/k8s/SeldonDeploymentStatusUpdateTest.java

@@ -6,6 +6,7 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.never;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -54,15 +55,33 @@ public void updateAvailableTest() throws IOException
 		supdate.updateStatus(selDepName, version, "test-deployment-fx-market-predictor-8e1d76f", 1, 1, namespace);
 
 		verify(mockCrdHandler,times(1)).getSeldonDeployment(eq(selDepName), eq(version), eq(namespace));
-		ArgumentCaptor<SeldonDeployment> argument = ArgumentCaptor.forClass(SeldonDeployment.class);
-		verify(mockCrdHandler,times(1)).updateSeldonDeploymentStatus(argument.capture());
+		verify(mockCrdHandler,times(1)).updateSeldonDeploymentStatus(any(SeldonDeployment.class));
+
+	}
+
+	@Test
+	public void updateAvailableOnFailedTest() throws IOException
+	{
+		createMocks();
 
-		SeldonDeployment sDepUpdated = argument.getAllValues().get(0);
-		Assert.assertEquals(1, sDepUpdated.getStatus().getPredictorStatusCount());
-		Assert.assertEquals(1, sDepUpdated.getStatus().getPredictorStatus(0).getReplicasAvailable());
-		Assert.assertEquals("Available", sDepUpdated.getStatus().getState());
+		String jsonStr = readFile("src/test/resources/model_failed.json",StandardCharsets.UTF_8);
+        SeldonDeployment sDep = SeldonDeploymentUtils.jsonToSeldonDeployment(jsonStr);
+
+		when(mockCrdHandler.getSeldonDeployment(any(String.class), any(String.class), any(String.class))).thenReturn(sDep);
+
+		SeldonDeploymentStatusUpdate supdate = new SeldonDeploymentStatusUpdateImpl(mockCrdHandler, mockSeldonDeploymentController, props);
+
+		final String selDepName = "SeldonDep1";
+		final String version = "v1alpha1";
+		final String namespace = "seldon";
+
+		supdate.updateStatus(selDepName, version, "test-deployment-fx-market-predictor-8e1d76f", 1, 1, namespace);
+
+		verify(mockCrdHandler,times(1)).getSeldonDeployment(eq(selDepName), eq(version), eq(namespace));
+		verify(mockCrdHandler,never()).updateSeldonDeploymentStatus(any(SeldonDeployment.class));	
 	}
 
+
 	@Test
 	public void twoUpdatesTest() throws IOException
 	{
@@ -126,5 +145,7 @@ public void removeTest() throws IOException
 		Assert.assertEquals("", sDepUpdated.getStatus().getState());
 	}
 
+
+
 
 }

cluster-manager/src/test/resources/model_failed.json

@@ -0,0 +1,61 @@
+{
+    "apiVersion": "machinelearning.seldon.io/v1alpha3",
+    "kind": "SeldonDeployment",
+    "metadata": {
+        "creationTimestamp": "2019-04-04T14:33:08Z",
+        "generation": 1,
+        "labels": {
+            "app": "seldon"
+        },
+        "name": "seldon-model",
+        "namespace": "seldon",
+        "resourceVersion": "6700",
+        "selfLink": "/apis/machinelearning.seldon.io/v1alpha3/namespaces/seldon/seldondeployments/seldon-model",
+        "uid": "9101adcf-56e6-11e9-865f-ec5f8735c09f"
+    },
+    "spec": {
+        "name": "test-deployment",
+        "oauth_key": "oauth-key",
+        "oauth_secret": "oauth-secret",
+        "predictors": [
+            {
+                "componentSpecs": [
+                    {
+                        "spec": {
+                            "containers": [
+                                {
+                                    "imagePullPolicy": "IfNotPresent",
+                                    "image": "seldonio/mock_classifier:1.0",
+                                    "name": "classifier",
+                                    "resources": {
+                                        "requests": {
+                                            "memory": "1Mi"
+                                        }
+                                    }
+                                }
+                            ],
+                            "terminationGracePeriodSeconds": 1
+                        }
+                    }
+                ],
+                "graph": {
+                    "children": [],
+                    "endpoint": {
+                        "type": "REST"
+                    },
+                    "name": "classifier",
+                    "type": "MODEL"
+                },
+                "labels": {
+                    "version": "v1"
+                },
+                "name": "example",
+                "replicas": 1
+            }
+        ]
+    },
+    "status": {
+        "description": "Cannot find field: imagename in message k8s.io.api.core.v1.Container",
+        "state": "Failed"
+    }
+}

python/seldon_core/microservice.py

@@ -11,6 +11,7 @@
 from seldon_core.flask_utils import ANNOTATIONS_FILE
 import seldon_core.wrapper as seldon_microservice
 from typing import Dict, Callable
+from seldon_core.flask_utils import SeldonMicroserviceException
 
 logger = logging.getLogger(__name__)
 
@@ -70,7 +71,16 @@ def parse_parameters(parameters: Dict) -> Dict:
         if type_ == "BOOL":
             parsed_parameters[name] = bool(strtobool(value))
         else:
-            parsed_parameters[name] = type_dict[type_](value)
+            try:
+                parsed_parameters[name] = type_dict[type_](value)
+            except ValueError:
+                raise SeldonMicroserviceException(
+                    "Bad model parameter: " + name + " with value " + value + " can't be parsed as a " + type_,
+                    reason="MICROSERVICE_BAD_PARAMETER")
+            except KeyError:
+                raise SeldonMicroserviceException(
+                    "Bad model parameter type: " + type_ + " valid are INT, FLOAT, DOUBLE, STRING, BOOL",
+                    reason="MICROSERVICE_BAD_PARAMETER")
     return parsed_parameters
 
 
@@ -167,6 +177,7 @@ def main():
     log_level_num = getattr(logging, args.log_level.upper(), None)
     if not isinstance(log_level_num, int):
         raise ValueError('Invalid log level: %s', args.log_level)
+
     logger.setLevel(log_level_num)
     logger.debug("Log level set to %s:%s", args.log_level, log_level_num)
 
@@ -185,6 +196,9 @@ def main():
 
     # set log level for the imported microservice type
     seldon_microservice.logger.setLevel(log_level_num)
+    logging.getLogger().setLevel(log_level_num)
+    for handler in logger.handlers:
+        handler.setLevel(log_level_num)
 
     port = int(os.environ.get(SERVICE_PORT_ENV_NAME, DEFAULT_PORT))
 

python/tests/test_microservice.py

@@ -8,9 +8,12 @@
 import pytest
 from seldon_core.proto import prediction_pb2
 from seldon_core.proto import prediction_pb2_grpc
+import seldon_core.microservice as microservice
+from seldon_core.flask_utils import SeldonMicroserviceException
 import grpc
 import numpy as np
 import signal
+import unittest
 
 @contextmanager
 def start_microservice(app_location,tracing=False,grpc=False,envs={}):
@@ -116,6 +119,7 @@ def test_model_template_app_grpc(tracing):
         feedback = prediction_pb2.Feedback(request=request,reward=1.0)
         response = stub.SendFeedback(request=request)
 
+
 def test_model_template_app_tracing_config():
     envs = {"JAEGER_CONFIG_PATH":join(dirname(__file__), "tracing_config/tracing.yaml")}
     with start_microservice(join(dirname(__file__), "model-template-app"),tracing=True,envs=envs):
@@ -133,3 +137,17 @@ def test_model_template_app_tracing_config():
             "http://127.0.0.1:5000/send-feedback", params="json=%s" % data)
         response.raise_for_status()
         assert response.json() == {'data': {'ndarray': []}, 'meta': {}}
+
+
+def test_model_template_bad_params():
+
+    params = [join(dirname(__file__), "model-template-app"),"seldon-core-microservice","REST","--parameters",'[{"type":"FLOAT","name":"foo","value":"abc"}]']
+    with unittest.mock.patch('sys.argv',params):
+        with pytest.raises(SeldonMicroserviceException):
+            microservice.main()
+
+def test_model_template_bad_params_type():
+    params = [join(dirname(__file__), "model-template-app"),"seldon-core-microservice","REST","--parameters",'[{"type":"FOO","name":"foo","value":"abc"}]']
+    with unittest.mock.patch('sys.argv',params):
+        with pytest.raises(SeldonMicroserviceException):
+            microservice.main()