-
Notifications
You must be signed in to change notification settings - Fork 495
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-132865 / 25.04 / Clean up old validation for keychaincredential
, make API definitions more precise
#15392
Conversation
keychaincredential
, make API definitions more precisekeychaincredential
, make API definitions more precise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic!
type: str | ||
attributes: Secret[dict] | ||
"""Distinguishes this Keychain Credential from others.""" | ||
type: Literal["SSH_KEY_PAIR", "SSH_CREDENTIALS"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move type
inside attributes
and use the more native discriminator approach? Basically, the same thing that's already done for cloud credentials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would change the schema of most of our keychaincredential
endpoints which are used in too many places to make this change worth it given that using a discriminated union would only slightly make our validation more performant. If we make this change in the future it should be its own PR.
This PR has been merged and conversations have been locked. |
This will improve readability of our docs and move away from our old schema definitions/validation.
http://jenkins.eng.ixsystems.net:8080/job/tests/job/api_tests/2608/