Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork993
Description
Prompted by#1372
Right now several of our public API functions or methods have a reference to anUnsetType instantiated asUNSET in their signature. For example:
Lines 690 to 705 in589c6e0
| defrequest( | |
| self, | |
| method:str, | |
| url:URLTypes, | |
| *, | |
| content:RequestContent=None, | |
| data:RequestData=None, | |
| files:RequestFiles=None, | |
| json:typing.Any=None, | |
| params:QueryParamTypes=None, | |
| headers:HeaderTypes=None, | |
| cookies:CookieTypes=None, | |
| auth:typing.Union[AuthTypes,UnsetType]=UNSET, | |
| allow_redirects:bool=True, | |
| timeout:typing.Union[TimeoutTypes,UnsetType]=UNSET, | |
| )->Response: |
Here's the full list of public API objects and methods that refer toUnsetType:
Client.requestClient.streamClient.sendClient.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 egauth=None. The goal was to be able to distinguish between for example "auth was not passed" (indicating "use the defaultauth on the client"), and "auth was passed explicitly withNone" (indicating that the userreally does not want auth to be used).
But the problem is that now thisUNSET instance andUnsetType are part of the public API: if people want to override methods onClient and keep the default behavior, then they need to importUNSET and default to it in the method.
importhttpxfromhttpx._configimportUNSET# OopsclassMyClient(httpx.Client):defpost(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 tonot have to refer toUNSET.
But there's also a case to be made for us dropping that "sentinel value" idea entirely. For example, we could:
- Accept a
**kwargsand pullauthfrom there.- Pros: no need for a special sentinel value.
- Cons: we lose type hinting on
authandtimeout(at least until typed**kwargsare 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__"], andLiteralis Py3.8+ (or we need to addtyping_extensionsas 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.