-
Notifications
You must be signed in to change notification settings - Fork 180
feat(api): support an "experimental" api version #17668
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: edge
Are you sure you want to change the base?
Conversation
Add the ability to specify a protocol's API level as "experimental", and to specify API calls as experimental. The experimental API level is greater than any named API level. That means that - An api method that is experimental can only be used if the protocol requests the experimental api level - This will remain true as the max supported api version changes
@@ -445,11 +447,28 @@ class JsonConfig(BaseModel): | |||
schemaVersion: int | |||
|
|||
|
|||
def _api_version_deser(obj: Any) -> APIVersion: |
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.
Do you want to make this more strictly typed than Any
?
if inp == "experimental": | ||
return cls.as_experimental() | ||
elif inp == "1": | ||
return cls(major=1, minor=0, experimental=False) |
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.
Why is version 1
special like that? Do we want to generalize version numbers to \d+\(\.\d+\)?
?
b_maj = other[0] | ||
b_min = other[1] | ||
b_exp = other[2] if len(other) > 2 else False | ||
# when you do a <= b, interpreter calls a.__le__(b) |
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.
I'm a little nervous that you're defining le without the other operators. Like, are users supposed to know that you're not allowed to use <
, only <=
?
class APIVersion(NamedTuple): | ||
major: int | ||
minor: int | ||
experimental: bool = False |
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.
Out of curiosity, would this implementation be much simpler if you made experimental
the first field in the tuple, and then just relied on the regular tuple ordering?
Add the ability to specify a protocol's API level as "experimental", and to specify API calls as experimental.
The experimental API level is greater than any named API level. That means that
review requests
to come out of draft