Allow pydantic 2 in python client requested requirements #433
Conversation
seanshi-scale
requested review from
ian-scale,
song-william and
yunfeng-scale
seanshi-scale
changed the title
| gpus: int | ||
| memory: StorageSpecificationType | ||
| gpu_type: GpuType | ||
| gpu_type: Optional[GpuType] |
There was a problem hiding this comment.
just curious, why not default GpuType to None as well?
There was a problem hiding this comment.
The location in the code does specify a value for GpuType, and I think it's pretty reasonable to make us specify some value of GpuType. For most of the other ones I changed, there isn't a value passed in, so None seems like a reasonable default.
| gpus: int | ||
| memory: StorageSpecificationType | ||
| gpu_type: GpuType | ||
| gpu_type: Optional[GpuType] |
There was a problem hiding this comment.
hmm, why is this optional, will we deploy LLMs on CPU?
There was a problem hiding this comment.
offline discussion: we're gonna add some defaults for the model architectures, I'm gonna keep this as optional since we may want overriding of defaults at some point
There was a problem hiding this comment.
for posterity: the Optional comes from the argument to Model.create()
Pull Request Summary
Previous pr allowed pydantic 2 to be compatible with the client, however we didn't update the requirements to note this.
Test Plan and Usage Guide
Pip install -e and check which version of pydantic it asks for. It asks for
pydantic >= 1.10which is correct.