Skip to content
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

Set Bytes Json Schema to base64 #462

Closed
wants to merge 3 commits into from

Conversation

HonakerM
Copy link
Contributor

@HonakerM HonakerM commented Sep 8, 2023

What this PR does / why we need it:
This PR sets the pydantic bytes json schema to base64. This updates the auto-generated openapi docs to correctly list bytes fields as base64 formatted strings instead of raw binary strings.

Special notes for your reviewer:
I discovered this config while exploring the pydantic repo. The PR that created the config was only merged last week so it hasn't been released. I tested my changes with a local clone of pydantic and ConfigDict is backwards compatible so this should be safe to merge but for abundance of caution this PR can wait until an official version is released.

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@prashantgupta24
Copy link
Contributor

This is awesome, thanks @HonakerM ! Yeah we can wait for the official release!

@gabe-l-hart
Copy link
Collaborator

@HonakerM I tried to resolve the conflicts, but apparently my mental black execution failed. Can you run fmt.sh on the latest tip of this branch since it's in your fork?

@gabe-l-hart
Copy link
Collaborator

@HonakerM It looks like the test failures are legitimate (I was distracted by the warning about the invalid escape sequence). If this is still important, can you look into getting those fixed?

@HonakerM
Copy link
Contributor Author

@gabe-l-hart Okay I'll take a look! This change isn't the most pressing so I'm not sure when I'll get to it but it'd probably be nice to have.

@gabe-l-hart
Copy link
Collaborator

@HonakerM Do you still need this one? It looks like it could use a rebase and some test fixes if so

HonakerM and others added 3 commits January 4, 2024 11:13
Signed-off-by: Michael Honaker <Michael.Honaker@ibm.com>
Signed-off-by: Gabe Goodhart <ghart@us.ibm.com>
Signed-off-by: Michael Honaker <Michael.Honaker@ibm.com>
@HonakerM
Copy link
Contributor Author

HonakerM commented Jan 4, 2024

@gabe-l-hart it's not required but would be nice. I just rebased/updated version, I'll keep an eye out for test failures

@gabe-l-hart
Copy link
Collaborator

Closing as stale. @HonakerM we can revisit later if this becomes urgent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants