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.

Sign up for GitHub

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

Jump to bottom

fix: compatibility with pydantic v2 #3273

Closed
wants to merge 1 commit into from
Closed

fix: compatibility with pydantic v2 #3273

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 9 additions & 8 deletions python/kserve/kserve/protocol/rest/v2_datamodels.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from typing import Optional, List, Union, Dict

import orjson
from pydantic import BaseModel
from pydantic import BaseModel, ConfigDict
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we surround the imports with try-catch in for backwards compatibility purposes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do -- we should likely also likely add if/else for the the class definitions to switch from the nested Config class to model_config


# TODO: in the future, this file can be auto generated
# https://pydantic-docs.helpmanual.io/datamodel_code_generator/
Expand Down Expand Up @@ -102,7 +102,7 @@ class ModelMetadataResponse(BaseModel):
}
"""
name: str
versions: Optional[List[str]]
versions: Optional[List[str]] = None
platform: str
inputs: List[MetadataTensor]
outputs: List[MetadataTensor]
Expand Down Expand Up @@ -136,7 +136,7 @@ class RequestInput(BaseModel):
name: str
shape: List[int]
datatype: str
parameters: Optional[Parameters]
parameters: Optional[Parameters] = None
data: List


Expand All @@ -150,7 +150,7 @@ class RequestOutput(BaseModel):
}
"""
name: str
parameters: Optional[Parameters]
parameters: Optional[Parameters] = None


class ResponseOutput(BaseModel):
Expand All @@ -168,7 +168,7 @@ class ResponseOutput(BaseModel):
name: str
shape: List[int]
datatype: str
parameters: Optional[Parameters]
parameters: Optional[Parameters] = None
data: List


Expand All @@ -183,7 +183,7 @@ class InferenceRequest(BaseModel):
"outputs" : [ $request_output, ... ] #optional
}
"""
id: Optional[str]
id: Optional[str] = None
parameters: Optional[Parameters] = None
inputs: List[RequestInput]
outputs: Optional[List[RequestOutput]] = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to replace Config with model_config if we want it to work in pydantic 2

Expand Down Expand Up @@ -229,10 +229,11 @@ class InferenceResponse(BaseModel):
}
"""
model_name: str
model_version: Optional[str]
model_version: Optional[str] = None
id: str
parameters: Optional[Parameters]
parameters: Optional[Parameters] = None
outputs: List[ResponseOutput]
model_config = ConfigDict(protected_namespaces='')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the model_config ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new syntax for pydantic 2 that's analogous to the nested Config class in pydantic 1.

My guess is that Pydantic2 uses "model_" a protected namespace which we need for model_version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to suppress this warning:

E   UserWarning: Field "model_name" has conflict with protected namespace "model_".
E
E   You may be able to resolve this warning by setting `model_config['protected_namespaces'] = ()`.

So it really ought to be an empty tuple.


class Config:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pydantic 1 syntax.

We should move the schema_extra into the json_schema_extra field in the model_config above.

https://docs.pydantic.dev/latest/migration/#changes-to-config

schema_extra = {
Expand Down