-
Notifications
You must be signed in to change notification settings - Fork 4
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
experiment: Use TypedDict for team_info response #10
base: main
Are you sure you want to change the base?
Conversation
This is the miniumum version that backports `NotRequired`, which we need for responses where some keys are not always present.
0d8a53c
to
cc10d9a
Compare
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.
Thanks for starting this 👍
Yeah, this would definitely be cool to add. If your goal is to fully schema out the API I wonder if we should look at pydantic
(I hear there's a flask-pydantic
too) or something similar to enable introspection? (I don't know what tooling there is for introspection TypedDict
s, if there is tooling then it's probably ok to stick with them).
If there's an easy upgrade path from TypedDict
s towards that sort of thing then it'd be a great step to do this first and then we can look at adding that later. If that upgrade is less trivial it's probably worth exploring the options first.
Being able to attach e.g: docstrings to the various members and have those documentation also form part of the API schema/docs would be amazing too. I've worked with that sort of approach for GraphQL schemata and it works really well there. I don't know what support there is for that sort of thing with non-GraphQL stuff though (?). If there's decent tooling around that it would also be great to consider options which allow us to add that on later (definitely no pressure to document everything as part of this!).
Are there other features you'd want to get out of a full API schema?
(I've left some minor comments inline, but perhaps let's ignore them until we've pinned down what sort of typing it's worth adding).
@@ -0,0 +1,31 @@ | |||
from __future__ import annotations |
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.
minor: I've tended to call files such as this types.py
(the core srcomp
project has such a file).
|
||
|
||
class NameWithURL(TypedDict): | ||
|
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.
nit: I'd rather not have blank lines at the top of classes please.
I'd be interested to see what you think of this @PeterJCLaw, it's half a step towards defining a full schema for the API. I have only implemented for one endpoint as I don't want to spend the time doing all of the endpoints if this isn't something you'd be interested in merging...