Skip to content

Conversation

medaminezghal
Copy link
Contributor

Update tests to be compatible with pydantic>=2.11

@medaminezghal
Copy link
Contributor Author

@medaminezghal
Copy link
Contributor Author

@bhosmer-ant Would you like to review this PR?

Copy link
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

We can update the lockfile and the tests, but we can't update the pyproject.toml.

@medaminezghal medaminezghal requested a review from Kludex June 27, 2025 13:20
@medaminezghal
Copy link
Contributor Author

@Kludex Is everything good now?

@medaminezghal
Copy link
Contributor Author

@Kludex I think it can be merged now

@Kludex Kludex merged commit 7942184 into modelcontextprotocol:main Jun 27, 2025
17 of 18 checks passed
@bhosmer-ant
Copy link
Contributor

@Kludex @medaminezghal unfortunately the changes here are incompatible with the older versions of pydantic we still want to support (due to pydantic-core changes from 2.27 to 2.33). I'll try to forward fix but may need to revert

@Kludex
Copy link
Member

Kludex commented Jun 27, 2025

@Kludex @medaminezghal unfortunately the changes here are incompatible with the older versions of pydantic we still want to support (due to pydantic-core changes from 2.27 to 2.33). I'll try to forward fix but may need to revert

You need to check the comments in this PR.

This PR doesn't bump the minimum version of pydantic. It bumps on this project pipeline only.

@bhosmer-ant
Copy link
Contributor

@Kludex @medaminezghal unfortunately the changes here are incompatible with the older versions of pydantic we still want to support (due to pydantic-core changes from 2.27 to 2.33). I'll try to forward fix but may need to revert

You need to check the comments in this PR.

This PR doesn't bump the minimum version of pydantic. It bumps on this project pipeline only.

Yup I'd read the comments. The problem is that we're not bumping the minimum version - it means that we're allowing older versions of pydantic with which the fixes here are incompatible. (The issue isn't with pydantic itself but with pydantic-core: the new version of pydantic pins a version of pydantic-core which has the changes that necessitated these changes.)

It will bother a lot of package maintainers that depend on versions in between.

Can you elaborate? Are there concrete instances you're aware of, or was it just on general principles?

@medaminezghal
Copy link
Contributor Author

@bhosmer-ant This change will only affect how tests work with Pydantic>=2.11

I don't think it will cause any problem with other packages.

@medaminezghal medaminezghal deleted the update-pydantic branch June 27, 2025 20:41
@bhosmer-ant
Copy link
Contributor

bhosmer-ant commented Jun 27, 2025

@bhosmer-ant This change will only affect how tests work with Pydantic>=2.11

I don't think it will cause any problem with other packages.

@medaminezghal the problem is when Pydantic < 2.11 is installed (as allowed by pyproject.toml). Earlier versions pin an earlier version of pydantic-core, under which the is not None check is redundant (typechecker warning) and the generated schema in the test is the earlier one (so it fails with this change).

The root of the problem is pyproject.toml - we'll need to narrow the versions of pydantic so that we're using only one version of pydantic-core. (And then sync uv.lock with it.)

@Kludex
Copy link
Member

Kludex commented Jun 28, 2025

I don't understand the problem here.

If this repository wants to the right thing, it needs to test the lowest and the latest version allowed. The pydantic version on the lockfile was already far above the minimum before this PR.

Python package repositories usually make sure the latest release are tested without breaking the previous versions - or test lowest and latest.


About my comment on the "other maintainers" I meant that people may be pinning pydantic for some reason, and it forces them to work and update the project, which is annoying.

gspencergoog pushed a commit to gspencergoog/mcp-python-sdk that referenced this pull request Jul 29, 2025
saqadri pushed a commit to saqadri/stdio-fixes that referenced this pull request Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants