Description
Prompted by #1372
Right now several of our public API functions or methods have a reference to an UnsetType
instantiated as UNSET
in their signature. For example:
Lines 690 to 705 in 589c6e0
Here's the full list of public API objects and methods that refer to UnsetType
:
Client.request
Client.stream
Client.send
Client.get
,Client.post
,Client.put
,Client.patch
,Client.delete
,Client.options
,Client.head
- Equivalent methods on
AsyncClient
Timeout(...)
This style was introduced in preference to eg auth=None
. The goal was to be able to distinguish between for example "auth
was not passed" (indicating "use the default auth
on the client"), and "auth
was passed explicitly with None
" (indicating that the user really does not want auth to be used).
But the problem is that now this UNSET
instance and UnsetType
are part of the public API: if people want to override methods on Client
and keep the default behavior, then they need to import UNSET
and default to it in the method.
import httpx
from httpx._config import UNSET # Oops
class MyClient(httpx.Client):
def post(self, *args, auth=UNSET, **kwargs):
... # Perhaps pre-process `auth`
It's fair to ask whether those use cases are valid, and if people can't always find a way to not have to refer to UNSET
.
But there's also a case to be made for us dropping that "sentinel value" idea entirely. For example, we could:
- Accept a
**kwargs
and pullauth
from there.- Pros: no need for a special sentinel value.
- Cons: we lose type hinting on
auth
andtimeout
(at least until typed**kwargs
are a thing, for example: Allow using TypedDict for more precise typing of **kwds python/mypy#4441) and autocompletion.
- Use a builtin instead, for example
auth="__UNSET__"
.- Pros: builtin type (string) means it removes the "I need to import
UNSET
" problem. - Cons:
- Still a sentinel value which users need to be aware of.
- To type this properly we'd need
Literal["__UNSET__"]
, andLiteral
is Py3.8+ (or we need to addtyping_extensions
as a dependency).
- Pros: builtin type (string) means it removes the "I need to import
I'm not sure what option is better, or if there are other options to consider, but I'm flagging this for 1.0 because this touches the public API.