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

NAS-133728 / 25.10 / Add NotRequired default value to our API #15462

Merged
merged 15 commits into from
Jan 30, 2025

Conversation

creatorcary
Copy link
Contributor

Pydantic does not allow named fields to be omitted entirely from models; in order for a field to be not required, it must have a default. NotRequired default value is a workaround to the problem. Any fields that are set to NotRequired will be excluded from model serialization.

Example:

class A(BaseModel):
    a: int
    b: int = 4
    c: int = NotRequired

A(a=1).model_dump()
# {'a': 1, 'b': 4}
A(a=1, c=2).model_dump()
# {'a': 1, 'b': 4, 'c': 2}

http://jenkins.eng.ixsystems.net:8080/job/tests/job/api_tests/2702/

@creatorcary creatorcary requested review from themylogin and a team January 22, 2025 22:28
@bugclerk bugclerk changed the title Add NotRequired default value to our API NAS-133728 / 25.04 / Add NotRequired default value to our API Jan 22, 2025
@bugclerk
Copy link
Contributor

@truenas truenas deleted a comment from bugclerk Jan 23, 2025
@yocalebo yocalebo changed the title NAS-133728 / 25.04 / Add NotRequired default value to our API NAS-133728 / 25.10 / Add NotRequired default value to our API Jan 28, 2025
@yocalebo
Copy link
Contributor

ping @themylogin what do you think? I like this idea. I believe it will become valuable when we start migrating APIs like our network plugin 😬

Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

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

Do integration tests pass with these changes?

src/middlewared/middlewared/api/base/model.py Outdated Show resolved Hide resolved
@creatorcary creatorcary merged commit 569b3a0 into master Jan 30, 2025
2 checks passed
@creatorcary creatorcary deleted the not-required branch January 30, 2025 17:22
@bugclerk
Copy link
Contributor

JIRA ticket https://ixsystems.atlassian.net/browse/NAS-133728 is targeted to the following versions which have not received their corresponding PRs: 25.10

@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Jan 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants