Repository navigation
TypeErrors against public interfaces. #122
Description
Partly prompted by this comment.
I think we should probably be a bit strict about enforcing types passed to our public interfaces. It'd be nice to raise loud clear errors, rather than failing with some arbitrary AttributeError in the middle of the codebase if a user passes something completely invalid to the interface. This would be stricter than what most Python libraries do, but I think it might be a good trade-off for us, to have a very strict and clearly demarked API.
That'd be easy enough to do since there's not too many points we'd need to enforce.
The big obvious one would be checking everything in AsyncClient.request(), which most of the rest of the public API calls into.
The would be some other bits & pieces to check too eg. Enforce that streaming data iterates through bytes/str, and not any other types. Enforce that dispatchers really do return a response type. Type checking on the various models that we expose.
Perhaps I'm being over-zelaous here? Any thoughts?
Activity
sethmlarson commentedon Jul 15, 2019
I'd rather be strict about what we accept, then we don't have to handle odd-ball cases down the road as a point of "backwards-compatibility" because it used to work but not on purpose.
lovelydinosaur commentedon Jul 15, 2019
Sounds good to me.
halbow commentedon Jul 28, 2019
I could take a look at this with ! :) I'll probably start with Client.request and AsynClient.request, do you have an idea/thoughts on the implementation ? (I was thinking that a decorator checking func.annotations could maybe do the trick)
sethmlarson commentedon Jul 28, 2019
Yeah start with a small example that Tom and I can take a look at for implementation review before we start applying it to all our interfaces. Are there any examples of this in another library?
lovelydinosaur commentedon Jul 29, 2019
Indeed - I reckon a
@typecheckeddecoartor approach is worth trying out here.We can keep it faily limited - it only needs to deal with the cases that we want to use it with.
Wrt. implementation I'd suggest starting out by only handling checking of actual types, and treating any
typing.Union/typing.Optional/typing.Dictcases all asAny.Once you've got it working with basic types you could then look at expanding it out to deal with the more complex composite cases.
lovelydinosaur commentedon Dec 11, 2019
We might want to have this at some point, but closing it as out-of-scope for now.