fix: Downgrade CI requirements mcp pin to satisfy pyproject constraint - #6713
fix: Downgrade CI requirements mcp pin to satisfy pyproject constraint#6713nithin42 wants to merge 1 commit into
Conversation
|
@nithin42 thanks for picking this up. I verified the fix locally, since the job it repairs can't run here yet: this PR shows 1 check passing and 4 skipped, so Both halves of the change hold up under The exact call that fails today is accepted. Dropping One incidental note, no action needed: @ntkathole @franciscojavierarceo this needs |
|
@ntkathole @franciscojavierarceo — could you please approve the 16 pending workflow runs on this PR? The This fix unblocks the |
9507eae to
05c5218
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6713 +/- ##
=======================================
Coverage 46.81% 46.81%
=======================================
Files 415 415
Lines 50399 50399
Branches 7214 7214
=======================================
Hits 23592 23592
Misses 25157 25157
Partials 1650 1650
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
@nithin42 the workflows are running now, and two have already failed for a reason unrelated to the mcp fix. Both The cause is the Fix is Two things that cost me time when I hit this on #6683, in case they save you some: Nothing local catches it. Unit tests, ruff and mypy all run under
If you don't have pixi installed, this container invocation works:
Worth noting the |
05c5218 to
32a3619
Compare
|
Thanks @larrysingleton007 — reverted the This PR is now narrowly focused on the mcp pin only:
The @jyejare — force-pushed to revert the incidental psutil change. The PR is now clean and narrowly focused on the mcp pin. Please approve the workflow runs again. Thanks! |
32a3619 to
0f1d8d4
Compare
|
@nithin42 Did you run |
|
@jyejare yes, I successfully ran However, doing so required adding a Because we modified Would you prefer that we:
|
|
I would go for option 2. |
6cd7618 to
e8e62e4
Compare
c0f3176 to
7edfce9
Compare
The CI requirements lock files pinned mcp==2.0.0 and mcp-types==2.0.0,
which conflicts with the pyproject.toml constraint mcp>=1.0,<2 added in
a recent change. This caused integration tests and the feature server to
fail with:
TypeError: Server.__init__() takes 2 positional arguments but 3 were given
This commit:
- Downgrades mcp to 1.29.0 across all nine py3.{10,11,12}-{ci,minimal,minimal-sdist}-requirements.txt lock files.
- Adds httpx-sse==0.4.0 with sha256 hashes to all nine lock files (required by mcp==1.29.0).
- Removes mcp-types from all lock files; mcp==1.29.0 declares no dependency on it.
- Updates unit tests in sdk/python/tests/unit/infra/feature_servers/test_mcp_server.py to isolate RestRegistryServer from unmocked RegistryServer gRPC handlers.
Fixes feast-dev#6706
Signed-off-by: Nithin <kumbam.nithingoud@gmail.com>
1a298ab to
4362125
Compare
|
Hi @jyejare @ntkathole @franciscojavierarceo @larrysingleton007, Quick update: I have rebased on the latest master and squashed the branch into a single clean commit. Key changes included:
All 23 CI checks (unit tests on Python 3.10/3.11/3.12 across Ubuntu & macOS, integration tests, and linters) are now fully green. The PR is ready for your review and merge if everything looks good to you (LGTM). Thanks! |
What this PR does / why we need it
A recent change added the
mcp>=1.0,<2constraint topyproject.tomlbut the CI dependencies requirements lock files still pinnedmcp==2.0.0andmcp-types==2.0.0. This version mismatch broke integration tests and crashed the feature server with aTypeError: Server.__init__() takes 2 positional arguments but 3 were givenduring CI runs.This PR downgrades
mcpto1.29.0(with updated hashes) and removesmcp-typesfrom all requirements files. We also loosenpsutildependency inpyproject.tomlto allow prebuilt binary wheels on Windows Python 3.11.Which issue(s) this PR fixes
Fixes #6706
Checks
git commit -s)