Fix #226: resolve Pydantic v1 recursion in RunDetails instantiation - #237
Conversation
|
@microsoft-github-policy-service agree |
d8dbb6c to
288014d
Compare
…tiation RunDetails() was triggering recursive __init__ wrapping in Pydantic v1 when instantiated repeatedly in server.py. Applied the same fix pattern already used in orchestrator.py by the original authors. Fixes microsoft#226 Co-authored-by: nik464 <nikhil18chaudhary@gmail.com>
288014d to
a116065
Compare
Renato L. de F. Cunha (renatolfc)
left a comment
There was a problem hiding this comment.
The current RunDetails cache does not fix the recursion: DataclassJSONEncoder repeatedly decorates an already decorated RunConfig and cumulatively wraps its initializer. The root fix is to reuse the class-owned pydantic_model. I opened a minimal patch with a 2,000-round regression test against your branch at Ashut0sh-mishra#1. Please merge that PR; upstream #237 will update automatically while preserving your attribution, and we can review the corrected head.
|
Addressed review feedback on this PR with the root fix and regression coverage. Changes made:
Updated files:
Notes on validation:
|
|
Renato L. de F. Cunha (@renatolfc) pushed requested fixes to this branch (root pydantic_model reuse + regression test). Could you please re-review when you have time? |
Summary
Fixes #226
Problem
The FarmVibes.AI Python client throws a RecursionError
when triggering workflows on the remote AKS cluster.
Root cause: RunDetails() was triggering recursive init
wrapping in Pydantic v1 when instantiated repeatedly in
server.py. Each instantiation wrapped the previous init,
causing a stack overflow on repeated workflow calls.
Fix
Applied the same fix pattern already present in
orchestrator.py (by the original authors) — cached an empty
RunDetails instance using asdict() and copy() instead of
creating a new RunDetails() on every call, avoiding the
recursive Pydantic wrapping entirely.
Files Changed
Validation
Co-authors
Co-authored-by: nik464 nikhil18chaudhary@gmail.com