Skip to content

Commit

Permalink
Improve kserve protocol version handling (pytorch#2957)
Browse files Browse the repository at this point in the history
* fix(kserve): ensure there's a default protocol configured

The current implementation retrieves the protocol to use from the
"PROTOCOL_VERSION" environment variable however there's no default value
which will trigger an error when served through Kserve as the base class
does a protocol check in the predict method that will fail with None.

The default protocol uses the same value as the base class.

* fix(kserve): ensure the protocol version configured is a valid value

* feat(kserve): make configuration file path configurable

This will allow to make the wrapper easier to test beside making
it possible to to change where the file should be looked for.

* test: add KServe wrapper test

* test(kserve_wrapper): add protobuf code generation

* fix(kserse): add None handling to the wrapper

In case None is passed, keep the default value set in the base __init__.

This makes TorchserveModel behave in the same fashion as the base class.

* refactor(test): rewrite wrapper test to be more complete

It now validates that the protocol version is passed properly as well
as failure if an invalid protocol is given.

* test: remove kserve pytest tests

The overhead required to run the tests outweighs it benefits.

The thing to keep in mind is that the fix allows to run models
through kserve deployments prior to 0.11.1.
  • Loading branch information
sgaist authored and andyi2it committed Mar 1, 2024
1 parent 3f8abe9 commit 3b92fc0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
5 changes: 4 additions & 1 deletion kubernetes/kserve/kserve_wrapper/TorchserveModel.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ def __init__(
self.inference_address = inference_address
self.management_address = management_address
self.model_dir = model_dir
self.protocol = protocol

# Validate the protocol value passed otherwise, the default value will be used
if protocol is not None:
self.protocol = PredictorProtocol(protocol).value

if self.protocol == PredictorProtocol.GRPC_V2.value:
self.predictor_host = grpc_inference_address
Expand Down
11 changes: 7 additions & 4 deletions kubernetes/kserve/kserve_wrapper/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import kserve
from kserve.model_server import ModelServer
from TorchserveModel import TorchserveModel
from TorchserveModel import PredictorProtocol, TorchserveModel
from TSModelRepository import TSModelRepository

logging.basicConfig(level=kserve.constants.KSERVE_LOGLEVEL)
Expand All @@ -15,7 +15,7 @@
DEFAULT_GRPC_INFERENCE_PORT = "7070"

DEFAULT_MODEL_STORE = "/mnt/models/model-store"
CONFIG_PATH = "/mnt/models/config/config.properties"
DEFAULT_CONFIG_PATH = "/mnt/models/config/config.properties"


def parse_config():
Expand All @@ -29,8 +29,11 @@ def parse_config():
"""
separator = "="
keys = {}
config_path = os.environ.get("CONFIG_PATH", DEFAULT_CONFIG_PATH)

with open(CONFIG_PATH) as f:
logging.info(f"Wrapper: loading configuration from {config_path}")

with open(config_path) as f:
for line in f:
if separator in line:
# Find the name and value by splitting the string
Expand Down Expand Up @@ -99,7 +102,7 @@ def parse_config():
model_dir,
) = parse_config()

protocol = os.environ.get("PROTOCOL_VERSION")
protocol = os.environ.get("PROTOCOL_VERSION", PredictorProtocol.REST_V1.value)

models = []

Expand Down

0 comments on commit 3b92fc0

Please sign in to comment.