-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle cyclic references in JSON encoding and schema #37
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
return _array_schema(obj) | ||
|
||
if id(obj) in seen: | ||
return {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return {} | |
return {'x-circular-reference': True} |
this way we can render something more nicely in the UI for circular references, rather than having it just look like a normal string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving this for now because it could make the overall schema significantly larger if the data consists only of JSON values which don't need a schema. This can be added if we actually decide to spend time on rendering in the UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though I don't know how much overhead this adds. Presumably we could do something in rust that would be faster, but I guess we probably don't want any native dependencies, so this makes sense to me.
de1e507
to
33f5348
Compare
Deploying logfire-docs with
|
Latest commit: |
33f5348
|
Status: | ✅ Deploy successful! |
Preview URL: | https://441c61ac.logfire-docs.pages.dev |
Branch Preview URL: | https://alex-recursive-json.logfire-docs.pages.dev |
Fixes bug seen in https://pydanticlogfire.slack.com/archives/C06EDRBSAH3/p1714397069462369