-
Notifications
You must be signed in to change notification settings - Fork 537
Upgrading pydantic to 2.11 #3472
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
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Documentation Link Check Results✅ Absolute links check passed |
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.
Just some questions otherwise LGTM if ur sure
pyproject.toml
Outdated
@@ -51,7 +51,7 @@ gitpython = "^3.1.18" | |||
packaging = ">=24.1" | |||
passlib = { extras = ["bcrypt"], version = "~1.7.4" } | |||
psutil = ">=5.0.0" | |||
pydantic = "~2.8" | |||
pydantic = "~2.11" |
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.
can we do >=2.x, <3
? @stefannica @bcdurak
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.
looking at the amount of manual fixes that Baris had to make to allow 2.11 to work, I would say no
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.
Most of the changes are coming from linting, however, if you take a look at models/__init__.py
, we had some changes related to the pydantic upgrade as well. I am not sure why this was not failing before, but if we would relax this 2 weeks ago, the release of pydantic 2.11 would make zenml
fail completely.
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.
We can alternatively have an upper limit, such as >=2.0, <=2.11
, but we would need to be careful how we handle breaking changes.
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.
@bcdurak can you try this out? it would be a big headache solver
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.
But what impact does that have on mlflow examples @stefannica ?
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.
@htahir1 absolutely no impact on any of our examples. This mlserver
feature is turned off by default and none of our examples are setting it explicitly. In fact, if you search mlserver
in all of the zenml-io GitHub org, you'll not find one mention of it in a demo, example or even notebook.
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.
@stefannica hmm wonder why we even have this then. im just a bit concerned about it , maybe it isn't mentioned explicitly but used in the e2e example?
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.
mlserver
is just one flavor of a local mlflow model server. The default one works just as well. I think this feature was never really used, even though it was available.
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.
ok then!
NLP template updates in |
Describe changes
zenml --help
zenml model list
zenml pipeline list
zenml stack --help
zenml stack list
This PR removes all
mlserver
related requirements from the MLflow integration because these are dragging in an older version of Pydantic. They are modeled as optional requirements going forward, that the user must install manually if needed.🎤 💧
Pre-requisites
Please ensure you have done the following:
develop
and the open PR is targetingdevelop
. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes