Add type annotations to everything referenced in __init__.py#771
Conversation
| def _make_handler( | ||
| url: str, | ||
| method: str, | ||
| timeout: Optional[float], |
There was a problem hiding this comment.
Seems fine to me, but most cases where I see Optional, there is typically a default of =None; but in this case I guess the caller of this is explicitly required to pass in an explicit None..
There was a problem hiding this comment.
Yeah, this one is an internal and requires that we explicitly pass timeout though even if the value is None (or a default value) from one of the other handlers. A bit unusual I agree.
| gateway: str, | ||
| job: str, | ||
| registry: CollectorRegistry, | ||
| grouping_key: Optional[Dict[str, str]] = None, |
There was a problem hiding this comment.
if grouping key is reused below, if it has a constant structure, may want to use a TypedDict
There was a problem hiding this comment.
I don't think it it has a constant structure, and TypedDict isn't available earlier than 3.8 anyway :(. I really want to use Protocol in a few places as well but cannot yet.
There was a problem hiding this comment.
It looks like you can use typing-extensions for TypedDict and Protocol.
There was a problem hiding this comment.
I have been avoiding adding typing-extensions as a dependency (we have no dependencies now), but if it becomes too painful I would considering adding it.
| job: str, | ||
| registry: CollectorRegistry, | ||
| grouping_key: Optional[Dict[str, str]] = None, | ||
| timeout: Optional[float] = 30, |
| @@ -169,10 +201,17 @@ def __init__(self, name, documentation, count_value=None, sum_value=None, labels | |||
| if labels is None: | |||
| labels = [] | |||
| self._labelnames = tuple(labels) | |||
There was a problem hiding this comment.
maybe self._labelnames = tuple(labels) if labels else ()
There was a problem hiding this comment.
Decent idea as a future cleanup, going to leave it out of this PR to keep the focus on types clear :).
csmarchbanks
left a comment
csmarchbanks
left a comment
There was a problem hiding this comment.
Thanks for the review!
| gateway: str, | ||
| job: str, | ||
| registry: CollectorRegistry, | ||
| grouping_key: Optional[Dict[str, str]] = None, |
There was a problem hiding this comment.
I don't think it it has a constant structure, and TypedDict isn't available earlier than 3.8 anyway :(. I really want to use Protocol in a few places as well but cannot yet.
| def _make_handler( | ||
| url: str, | ||
| method: str, | ||
| timeout: Optional[float], |
There was a problem hiding this comment.
Yeah, this one is an internal and requires that we explicitly pass timeout though even if the value is None (or a default value) from one of the other handlers. A bit unusual I agree.
| @@ -169,10 +201,17 @@ def __init__(self, name, documentation, count_value=None, sum_value=None, labels | |||
| if labels is None: | |||
| labels = [] | |||
| self._labelnames = tuple(labels) | |||
There was a problem hiding this comment.
Decent idea as a future cleanup, going to leave it out of this PR to keep the focus on types clear :).
csmarchbanks
force-pushed
the
type-everything-from-init
branch
from
28bc122 to
60bc25d
Compare
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
csmarchbanks
force-pushed
the
type-everything-from-init
branch
from
60bc25d to
7790582
Compare
Also update built in collectors to use the new Abstract Base Class. Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
csmarchbanks
force-pushed
the
type-everything-from-init
branch
from
7790582 to
35a48c2
Compare
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
csmarchbanks
force-pushed
the
type-everything-from-init
branch
from
35a48c2 to
b3a00ea
Compare
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
csmarchbanks
force-pushed
the
type-everything-from-init
branch
from
f277116 to
c350e5c
Compare
3 participants
Fixes: #760
This should cover most of the public surface area used by others. There is more typing work to be done before we can enable stricter types, but this will be a good next step.