fix(sdk): prefer model_dump_json() over json() in JSONEncoder for Pydantic v2#3770
fix(sdk): prefer model_dump_json() over json() in JSONEncoder for Pydantic v2#3770s-zx wants to merge 2 commits intotraceloop:mainfrom
Conversation
…antic v2
Pydantic v2 deprecated the .json() method in favour of .model_dump_json().
Calling .json() on a Pydantic v2 BaseModel instance emits:
PydanticDeprecatedSince20: The 'json' method is deprecated;
use 'model_dump_json' instead.
Fix: check for .model_dump_json() first (Pydantic v2 path) before falling
back to .json() (Pydantic v1 path). This eliminates the deprecation warning
without breaking Pydantic v1 compatibility.
Fixes traceloop#3516
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdate to JSON encoder fallback ordering: prefer Pydantic v2 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/traceloop-sdk/traceloop/sdk/utils/json_encoder.py`:
- Around line 17-23: The JSONEncoder.default implementation currently uses
Pydantic's model_dump_json() and .json() which return JSON strings and cause
double-encoding; change the branches in JSONEncoder.default to use model_dump()
for Pydantic v2 and .dict() for Pydantic v1 so the method returns native Python
objects (dict/list) instead of serialized strings, i.e., replace checks for
hasattr(o, "model_dump_json") and hasattr(o, "json") to call o.model_dump() and
o.dict() respectively (falling back to existing behavior if those methods are
absent).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 749b32c5-a9cf-4fa4-87bc-ab969932854b
📒 Files selected for processing (1)
packages/traceloop-sdk/traceloop/sdk/utils/json_encoder.py
…()/json()
JSONEncoder.default() must return a JSON-serializable Python object (dict,
list, etc.) that the encoder will then serialise. Both model_dump_json()
(Pydantic v2) and .json() (Pydantic v1) return an already-serialised JSON
string; when returned from default() that string is re-serialised as a JSON
string literal, producing double-escaped output like
"{\"name\": \"John\"}"
instead of
{"name": "John"}.
Use model_dump() (Pydantic v2) and .dict() (Pydantic v1) instead, which
return plain Python dicts that the encoder serialises correctly. This also
avoids the PydanticDeprecatedSince20 warnings that motivated the previous
change, because model_dump() is the current recommended API.
|
Good catch — fixed. Changed to |
2 participants
Problem
JSONEncoder.default()callso.json()when serialising Pydantic model instances. In Pydantic v2,.json()is deprecated in favour of.model_dump_json(), causing a large volume ofPydanticDeprecatedSince20warnings:Any workflow or task decorated with
@task/@workflowwhose arguments or return values are Pydantic models triggers this warning.Reported in #3516.
Fix
Check for
.model_dump_json()first (Pydantic v2 path) before falling back to.json()(Pydantic v1 path):This eliminates the deprecation warnings for Pydantic v2 models without breaking backwards compatibility with Pydantic v1.
Fixes #3516
Summary by CodeRabbit