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
Upgrade to Pydantic V2 #650
base: main
Are you sure you want to change the base?
Upgrade to Pydantic V2 #650
Conversation
✅ Deploy Preview for continuedev canceled.
|
@@ -3,7 +3,8 @@ | |||
from typing import List, Optional, cast | |||
|
|||
from aiohttp import ClientPayloadError | |||
from openai import error as openai_errors | |||
import openai |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uplifted openai v0.27.5 to v1.3.6 which was a breaking change.
As mentioned we'll need to uplift the entire OpenAI framework but it's a lot easier to implement
params: Optional[Dict] = {} | ||
|
||
# Allow step class for the migration | ||
@validator("step", pre=True, always=True) | ||
@field_validator("step") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pydantic Docs:
validator has been deprecated, and should be replaced with field_validator
The new @field_validator decorator does not have the each_item keyword argument; validators you want to apply to items within a generic container should be added by annotating the type argument. See validators in Annotated metadata for details. This looks like List[Annotated[int, Field(ge=0)]]
https://docs.pydantic.dev/latest/migration/#validator-and-root_validator-are-deprecated
class Config: | ||
arbitrary_types_allowed = True | ||
exclude = {"ide", "delete_documents", "update_documents"} | ||
model_config = ConfigDict(arbitrary_types_allowed=True, exclude={"ide", "delete_documents", "update_documents"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was updated by Pydantic's bump-pydantic server
run
Pydantic Doc:
The Pydantic V1 behavior to create a class called Config in the namespace of the parent BaseModel subclass is now deprecated.
https://docs.pydantic.dev/latest/migration/#changes-to-config
@@ -257,10 +258,22 @@ class SerializedContinueConfig(BaseModel): | |||
@staticmethod | |||
@contextmanager | |||
def edit_config(): | |||
config = SerializedContinueConfig.parse_file(CONFIG_JSON_PATH) | |||
# Read the JSON file and parse it into a dictionary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pydantic Docs:
In particular, parse_raw and parse_file are now deprecated. In Pydantic V2, model_validate_json works like parse_raw. Otherwise, you should load the data and then pass it to model_validate.
https://docs.pydantic.dev/latest/migration/#changes-to-pydanticbasemodel
server/continuedev/core/main.py
Outdated
|
||
class Config: | ||
smart_union = True | ||
# TODO[pydantic]: The following keys were removed: `smart_union`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Says smart_union is deprecated but no suggestion on how to migrate it
https://docs.pydantic.dev/latest/migration/#changes-to-config
|
||
class Config: | ||
copy_on_model_validation = False | ||
# TODO[pydantic]: The following keys were removed: `copy_on_model_validation`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy_on_model_validation deprecated but no replacment
https://docs.pydantic.dev/latest/migration/#changes-to-config
) | ||
def roles_not_none(cls, v, values): | ||
def roles_not_none(cls, v, val_info): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
value: ModelField is now ValidationInfo. I renamed the argument from val_info to make it clear it is not value.
if v is None: | ||
return values["default"] | ||
return cls.model_fields[val_info.field_name].default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no default in the val_info so it now needs to be fetched from the class.model_fields...
@@ -31,11 +31,13 @@ class LocalIdeProtocol(AbstractIdeProtocolServer): | |||
workspace_directory: str = os.getcwd() | |||
unique_id: str = get_mac_address() | |||
|
|||
filesystem: FileSystem = RealFileSystem() | |||
filesystem: FileSystem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its now fussy about not parsing in an arguement so this needs to be instantiated in the init
@validator("prompt_templates", pre=True, always=True) | ||
def set_prompt_templates(cls, prompt_templates, values): | ||
return prompt_templates or autodetect_prompt_templates(values["model"]) | ||
# TODO Pydantic V2 no longer supports fields it should be done inline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit Confused here becuase the fields exist in the parent and child Config class which has been deprecated. Happy to discuss about what you want to do here.
"*", | ||
pre=True, | ||
always=True, | ||
@field_validator( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how to handle this. Each fields now needs validate_default=True and to be Annotated. But I'm unsure which fields they are
display_title = "Diff" | ||
description = "Output of 'git diff' in current repo" | ||
dynamic = True | ||
title: str = "diff" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pydantic2 is fussy and everything needs a type now
boltons==23.0.0 | ||
pydantic==1.10.8 | ||
pydantic==2.0.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I came here for!
fastapi==0.95.2 | ||
typer==0.9.0 | ||
openai==0.27.5 | ||
openai==1.3.6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've re-ordered this file so it aligns with the pyproject.toml
@@ -33,12 +33,12 @@ | |||
ContextProviderDescription, | |||
] | |||
+ [SerializedContinueConfig, ModelDescription] | |||
+ [ModelProvider, ModelName] | |||
+ [ModelNameWrapper] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pydantic Doesn't support Literals Directly
pydantic/bump-pydantic#124
So we need to create a class wrapper which is then referenced in modelData.tx.
import { Provider } from "../schema/ModelDescription";
import { ModelName } from "../schema/ModelNameWrapper";
# NOTE [pydantic]: with Pydantic Unions don't use relative imports (i.e. `from .main import DeltaStep`) otherwise it will fail to match throwing: | ||
# Input should be a valid dictionary or instance of DeltaStep .... | ||
# The string class names will differ e.g.: | ||
# <class 'continuedev.core.main.DeltaStep'> != <class 'server.continuedev.core.main.DeltaStep'> | ||
update: "UpdateStep" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# NOTE [pydantic]: with Pydantic Unions don't use relative imports (i.e. `from .main import DeltaStep`) otherwise it will fail to match throwing:
# Input should be a valid dictionary or instance of DeltaStep ....
# The string class names will differ e.g.:
# <class 'continuedev.core.main.DeltaStep'> != <class 'server.continuedev.core.main.DeltaStep'>
Upgraded numerous modules comments will be inline to descripe all of the changes and decisions made