Reconsider UNSET?

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:

httpx/httpx/_client.py

Lines 690 to 705 in 589c6e0

    
           def request( 
        
               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 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 pull auth from there.
- Pros: no need for a special sentinel value.
- Cons: we lose type hinting on auth and timeout (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__"], and Literal is Py3.8+ (or we need to add typing_extensions as a dependency).

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.

I’m wondering if it is ok to use typing.Undefined

@lepture

@lepture Where does typing.Undefined exist? python/mypy#675 mentions it as an "old convention", and import typing; typing.Undefined is an AttributeError.

However, this makes mypy happy enough…

import builtins

class Client:
    def request(
        self,
        # ...
        auth: typing.Union[AuthTypes, builtins.ellipsis] = ...,
        allow_redirects: bool = True,
        timeout: typing.Union[TimeoutTypes, builtins.ellipsis] = ...,
    ) -> Response:

In otherwords, ... (Ellipsis, aka builtins.ellipsis) could be treated as a shared UNSET sentinel value.

I've seen FastAPI do this in places. In general FastAPI uses it to mean "this API parameter is required", whereas here we're giving it a different meaning). But it still means means it's not totally unprecedented in large typing-heavy projects.

And for users who want to override things then it looks reasonable enough…

import httpx

class MyClient(httpx.Client):
    def post(self, *args, auth=..., **kwargs):
        ...  # Perhaps pre-process `auth`

I've used ... (ellipsis) as a shortcut for undefined a few times at $WORK before, but to be honest it seemed a bit of "magic" (reviewers were confused initially, it's not an usual pattern) and it was never part of any public API.

What's wrong with having the current sentinel UNSET as part of the public API? It has no issues with mypy, and I see the only usages would be for libs that want to subclass httpx so it shouldn't be that widespread.

Yeah, I've just closed #1385 because builtins.ellipsis is actually invalid at runtime (raises AttributeError), it's only available to type checkers, and so most likely for their private use only.

@florimondmanca

@florimondmanca

We can specify

UnsetType = type(Ellipsis)

in _types.py

Then mypy would be able to accept Ellipsis as a sentinel value

Some thoughts here...

I think we could perfectly well drop it on Timeout(...) in favour of kwargs.
That seems to just make sense there to me somehow.

The remaining cases are auth and timeout arguments on client methods, where we have it because we necessarily need a value that means "Use the client defaults" which is distinct from None, meaning "no auth" or "no timeout". Perhaps we could expose our sentinel as public API for those cases, but renaming it slightly to "USE_CLIENT_DEFAULT", which is much more clear?

Im happy to open a PR for this one

Navigation Menu

Repository navigation

Reconsider UNSET? #1384

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	def request(
	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:

Reconsider UNSET? #1384

Description

Activity

lepture commented on Nov 7, 2020

florimondmanca commented on Nov 7, 2020

jcugat commented on Nov 7, 2020

florimondmanca commented on Nov 7, 2020

cdeler commented on Nov 9, 2020

lovelydinosaur commented on Nov 13, 2020

jkklapp commented on Apr 5, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions