fix(aenv): pin transitive deps to coexist #76
Conversation
….x + pydantic 2.11.x)
aenv's PyPI metadata previously left fastmcp / mcp / pydantic upper-bounds
open, so pip's resolver pulls:
* fastmcp 3.x → pydantic-settings>=2.5 (latest) → pydantic 2.13.x
* mcp >= 1.17 → pydantic-settings>=2.5.2 (forced)
That cascades to pydantic-core 2.41.x and breaks asap-service's
LLM gateway which still relies on openai 1.78.1 + pydantic 2.11.10
+ pydantic-core 2.33.2 (openai 2.x SSE schema is incompatible).
Pin four deps to ranges that coexist with the asap pin set without
sacrificing aenv's runtime contract:
* pydantic >= 2.7, < 2.12 (asap is on 2.11.10)
* pydantic-settings >= 2.0 (explicit so it isn't pulled by mcp alone)
* mcp >= 1.0, < 2 (block future 2.x ABI break)
* fastmcp >= 2.13, < 3 (block 3.x's stricter pydantic chain)
Local verification (asap-service Python env, OSX miniconda 3.12):
* pip install aenvironment-0.1.6.whl
-> openai 1.78.1 unchanged, pydantic 2.11.10 unchanged,
pydantic-core 2.33.2 unchanged, fastmcp resolves to 2.14.7.
* from aenv import Environment -> OK
* AEnvSandboxService._import_environment() -> aenv.core.environment.Environment
* pytest biz/flash_game_agent/tests -m 'not integration' -> 11 passed.
openai-agents stays under [project.optional-dependencies].agents.
There was a problem hiding this comment.
Code Review
This pull request updates dependency constraints in pyproject.toml, including a major version upgrade for pydantic and the addition of upper bounds for mcp and fastmcp. Reviewers suggested relaxing the pydantic upper bound to prevent dependency conflicts and questioned the necessity of adding pydantic-settings as a direct dependency if it is already a transitive one.
|
|
||
| dependencies = [ | ||
| "pydantic>=1.10.0", | ||
| "pydantic>=2.7,<2.12", |
There was a problem hiding this comment.
Pinning pydantic to a minor version upper bound (<2.12) is quite restrictive for a library and may cause dependency conflicts for downstream users who require newer versions of Pydantic (e.g., 2.12 or 2.13) in their own projects. If the intent is to avoid the pydantic-core 2.41 issue introduced in pydantic 2.13, consider using <2.13 to allow for more flexibility, unless 2.12 is also confirmed to be incompatible. Additionally, since moving from 1.10.0 to 2.7 is a major version jump that drops Pydantic v1 support, consider bumping the package version (e.g., to 0.2.0) to signal this breaking change to users.
| dependencies = [ | ||
| "pydantic>=1.10.0", | ||
| "pydantic>=2.7,<2.12", | ||
| "pydantic-settings>=2.0", |
There was a problem hiding this comment.
Adding pydantic-settings as a direct dependency seems redundant if it is not used directly by the aenv source code and is already pulled as a transitive dependency by mcp or fastmcp. Furthermore, without an upper bound, this entry does not help prevent the 'cascade' issue described in the PR if a newer version of pydantic-settings pulls in an incompatible pydantic version. If the goal is to control the version of this transitive dependency, an upper bound would be necessary, though the pin on pydantic itself (line 27) should already address the primary concern.
….x + pydantic 2.11.x)
aenv's PyPI metadata previously left fastmcp / mcp / pydantic upper-bounds open, so pip's resolver pulls:
That cascades to pydantic-core 2.41.x and breaks asap-service's LLM gateway which still relies on openai 1.78.1 + pydantic 2.11.10
Pin four deps to ranges that coexist with the asap pin set without sacrificing aenv's runtime contract:
Local verification (asap-service Python env, OSX miniconda 3.12):
openai-agents stays under [project.optional-dependencies].agents.