Python Wrapper does not adhere to Timeout annotations

[mwm5945]

The timeout annotations specified here don't appear to have any effect on the python wrapper:

seldon-core/python/seldon_core/microservice.py

Line 426 in f483313

"timeout": 5000,

seldon-core/python/seldon_core/wrapper.py

Line 289 in f483313

if ANNOTATION_GRPC_MAX_MSG_SIZE in annotations:

[mwm5945]

It doesn't look like a timeout on the GRPC server is an option: https://github.com/grpc/grpc/blob/v1.38.x/include/grpc/impl/codegen/grpc_types.h

[axsaucedo]

@mwm5945 from my understanding all unary calls from grpc are underneath still streaming calls, which means that the server would expect connections to remain open for the time that the client requests - this would then mean that the timeout would only be a configuration that would be set on the client side, which would be on the executor. Did you basically experience that you've set a timeout annotation and it wasn't being respected? If that is the case, then the issue would be in the executor, and we could have a look at why that is happening. Would you be able to confirm this point?

[mwm5945]

We're currently running REST, and yes, saw that the timeout wasn't being respected. I was just curious to see if the grpc timeout was also propagated or not :)

[ukclivecox]

The grpc timeout is added at present in the executor

seldon-core/executor/api/grpc/client.go

Lines 32 to 45 in f483313

	if annotations != nil {
	val := annotations[k8s.ANNOTATION_GRPC_TIMEOUT]
	if val != "" {
	timeout, err := strconv.Atoi(val)
	if err != nil {
	log.Error(err, "Failed to parse annotation to int so will ignore", k8s.ANNOTATION_GRPC_TIMEOUT, val)
	} else {
	dur := time.Millisecond * time.Duration(timeout)
	log.Info("Adding grpc timeout to client", "value", timeout, "seconds", dur)
	interceptors = append(interceptors, unaryClientInterceptorWithTimeout(dur))
	}
	}
	}

But its true that would just timeout the connection in the executor but not sure how it would effect a long running call in the python client that was ongoing. So I imagine the timeout needs also to be added there to cause an exception.

[axsaucedo]

@mwm5945 I have now added PR #3617 to ensure python also respects the timeouts - would be great if you can have a look