-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix mismatching type of ConsistencyPolicy
with docstring
#43150
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
base: main
Are you sure you want to change the base?
Fix mismatching type of ConsistencyPolicy
with docstring
#43150
Conversation
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.
Pull Request Overview
This PR fixes a type annotation mismatch for the ConsistencyPolicy
attribute in the DatabaseAccount
class. The docstring type hint and actual code implementation had inconsistent type annotations.
- Updates docstring type annotation from
Dict[str, Union[str, int]]
todict[str, str]
- Updates instance variable type annotation from
Optional[UserConsistencyPolicy]
toOptional[dict[str, str]]
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
:ivar ConsistencyPolicy: | ||
UserConsistencyPolicy settings. | ||
:vartype ConsistencyPolicy: Dict[str, Union[str, int]] | ||
:vartype ConsistencyPolicy: dict[str, str] |
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.
Is this always going to be dict[str, str]
? We don't want to change this later.
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.
Yes, the type will always be dict[str, str]
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.
Just to clarify, I was referring specifically to the value type - there's never going to be ints or booleans in this dictionary?
Unless something is a protocol that requires a mapping of strings, we usually stick with dict[str, Any]
.
But if your service mandates that these be string properties, then this is fine.
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.
LGTM
Description
The typehint for
ConsistencyPolicy
in docstring and actual code were mismatching. This fix will sync the typehints for them.All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines