Skip to content

fix(python): exclude unset API values on model dump #3777

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

Merged
merged 4 commits into from
Sep 19, 2024

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Sep 19, 2024

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/DI-2951

Changes included:

see https://algolia.slack.com/archives/C336MP4UA/p1726487274846109 for context

Pydantic requires default values on optional fields (which is an other problem I should tackle), but those default values are also added to the dict when dumping the model, which we don't want because we only want to rely on what the API returns, in case there's a change there before it is reflected on the client.

We now tell Pydantic to only rely on values that are set, and ignore the defaults.

You can try it by running the playground as-is, and see that the default searchParams are not provided, only the one explicit defined

@shortcuts shortcuts self-assigned this Sep 19, 2024
@shortcuts shortcuts requested a review from a team as a code owner September 19, 2024 10:03
@algolia-bot
Copy link
Collaborator

algolia-bot commented Sep 19, 2024

✔️ Code generated!

Name Link
🪓 Triggered by 5612bd99fef3f504e33dbb4955727593a907be05
🍃 Generated commit d1417ca6183f7f4d255cfbc0ba7d5e7a8a872065
🌲 Generated branch generated/fix/python-default-values
📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Req/s
javascript 1669
php 1547
csharp 1052
python 951
java 944
ruby 906
swift 754
go 560

@shortcuts shortcuts changed the title fix(python): do not set API default values fix(python): exclude default API values on model dump Sep 19, 2024
@shortcuts shortcuts force-pushed the fix/python-default-values branch from 3ad5b24 to 2a89472 Compare September 19, 2024 10:47
@shortcuts shortcuts changed the title fix(python): exclude default API values on model dump fix(python): exclude unset API values on model dump Sep 19, 2024
@shortcuts shortcuts marked this pull request as draft September 19, 2024 10:53
@shortcuts shortcuts marked this pull request as ready for review September 19, 2024 10:57
Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

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

nice

@@ -2,5 +2,5 @@ argcomplete==3.5.0
click==8.1.7
packaging==24.1
pipx==1.7.1
platformdirs==4.3.3
platformdirs==4.2.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

why the downgrade ? I think this file should be excluded from PR

Copy link
Member Author

Choose a reason for hiding this comment

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

idk I've let the tooling do its thing, it was after this change

Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

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

gg

@shortcuts shortcuts enabled auto-merge (squash) September 19, 2024 11:53
@shortcuts shortcuts merged commit ba745be into main Sep 19, 2024
28 checks passed
@shortcuts shortcuts deleted the fix/python-default-values branch September 19, 2024 12:17
algolia-bot added a commit that referenced this pull request Sep 19, 2024
algolia-bot added a commit to algolia/algoliasearch-client-python that referenced this pull request Sep 19, 2024
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.

3 participants