Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for various Python and Operator fixes #488

Merged
merged 5 commits into from
Apr 4, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,4 @@ doc/source/_static/cluster-manager
python/build/
python/dist/
testing/scripts/proto
python/.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -126,5 +145,7 @@ public void removeTest() throws IOException
Assert.assertEquals("", sDepUpdated.getStatus().getState());
}




}
61 changes: 61 additions & 0 deletions cluster-manager/src/test/resources/model_failed.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
Binary file removed python/.coverage
Binary file not shown.
16 changes: 15 additions & 1 deletion python/seldon_core/microservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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)

Expand All @@ -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))

Expand Down
18 changes: 18 additions & 0 deletions python/tests/test_microservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -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={}):
Expand Down Expand Up @@ -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):
Expand All @@ -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()