Skip to content

Fix Secret serialization schema, applicable for unions #9240

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 16, 2024
Merged

Fix Secret serialization schema, applicable for unions #9240

merged 2 commits into from
Apr 16, 2024

Conversation

sydney-runkle
Copy link
Contributor

@sydney-runkle sydney-runkle commented Apr 14, 2024 •
edited by pydantic-hooky bot
Loading

Fix #9231

Selected Reviewer: @hramezani

@sydney-runkle sydney-runkle mentioned this pull request Apr 14, 2024
Closed
1 task
@sydney-runkle sydney-runkle added the relnotes-fix Used for bugfixes. label Apr 14, 2024
@sydney-runkle
Copy link
Contributor Author

Please review

@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

cloudflare-workers-and-pages bot commented Apr 14, 2024 •
edited
Loading

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6673dc5
Status: ✅  Deploy successful!
Preview URL: https://8d35bcec.pydantic-docs2.pages.dev
Branch Preview URL: https://fix-secret-ser.pydantic-docs2.pages.dev

View logs

@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Apr 14, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #9240 will not alter performance

Comparing fix-secret-ser (6673dc5) with main (6aab43e)

Summary

✅ 13 untouched benchmarks

davidhewitt
davidhewitt reviewed Apr 16, 2024
View reviewed changes
Copy link
Contributor

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix here seems a bit magical to me, can you give a bit more detail on why the original schema was wrong? I wonder if there's a deeper bug or otherwise awkward behaviour...

@sydney-runkle
Copy link
Contributor Author

@davidhewitt,

I think there's a deeper bug, that the serialization attached to the individual python and json schemas wasn't working - definitely something that we could fix + use to clean up some schemas or at least make them a bit more intuitive.

Will report this in core.

@sydney-runkle sydney-runkle mentioned this pull request Apr 16, 2024
Open
@sydney-runkle
Copy link
Contributor Author

Opened an issue here to address that bug in core, but this should work for now. Here's that issue: pydantic/pydantic-core#1270

@sydney-runkle sydney-runkle merged commit ae71183 into main Apr 16, 2024
@sydney-runkle sydney-runkle deleted the fix-secret-ser branch April 16, 2024 14:23
@Danipulok Danipulok mentioned this pull request Jul 2, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhewitt davidhewitt davidhewitt left review comments

@hramezani hramezani hramezani approved these changes

Assignees

@hramezani hramezani

Labels
ready for review relnotes-fix Used for bugfixes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JSON serialisation error with Secret types union.
3 participants
@sydney-runkle @davidhewitt @hramezani