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

fix: ensure pydantic models serialize to json-compatible dicts. #346

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

dwinston
Copy link
Collaborator

@dwinston dwinston commented Nov 2, 2023

add regression test

closes #345

replaces many instances of pydantic-model-to-dict calls (.dict(...)) with .model_dump(mode="json",...) calls as per pydantic V2 guidance.

Copy link
Collaborator

@eecavanna eecavanna left a comment

Choose a reason for hiding this comment

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

I see the related section of the Pydantic v1 to v2 migration guide.

image

This branch also introduces some changesheet-related tests.

@eecavanna
Copy link
Collaborator

I recommend holding off on merging this until there is no risk of such a merge complicating the resolution of issue #348.

)
doc = drs_obj.dict(exclude_unset=True)
doc = drs_obj.model_dump(exclude_unset=True, mode="json")
Copy link
Collaborator

@PeopleMakeCulture PeopleMakeCulture Nov 2, 2023

Choose a reason for hiding this comment

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

I don't think order matters functionally, but this is stylistically inconsistent with the rest of the codebase

Copy link
Collaborator

@PeopleMakeCulture PeopleMakeCulture left a comment

Choose a reason for hiding this comment

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

approved with one none-blocking style comment. Waiting on resolution of #348 to merge

@dwinston dwinston merged commit 2c2a4dd into main Nov 2, 2023
dwinston added a commit that referenced this pull request Nov 3, 2023
@eecavanna eecavanna deleted the 345-pydantic-model-to-json branch November 13, 2023 18:25
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.

API endpoint /metadata/changesheets:submit returns HTTP 500 Internal Server Error
3 participants