Add BaseRequestHandler instance attributes

[jstasiak]

From the BaseRequestHandler documentation:

handle()

This function must do all the work required to service a request.
The default implementation does nothing. Several instance attributes
are available to it; the request is available as self.request; the
client address as self.client_address; and the server instance as
self.server, in case it needs access to per-server information.

The type of self.request is different for datagram or stream
services. For stream services, self.request is a socket object;
for datagram services, self.request is a pair of string and
socket.

I also added a union that expresses the fact that client_address can be
a string in case of using Unix domain sockets.

[gvanrossum]

Those unions are going to cause a world of pain, since most handler implementations are written assuming one type or the other but not both. I wonder if the class shouldn't be made generic over a type variable (or type variables?) defining the request and address types?

[jstasiak]

@gvanrossum That sounds sensible, see the updated patch. I didn't know how the type variables should be called and didn't want to just name them T and S, feel free to rename. Note that the build fails because of what I believe to be mypy bug (unless I misunderstood how inheritance works with generics and not all type variables set), reported here: python/mypy#1936.

[gvanrossum]

Looks like pytype doesn't like it if a type variable's a value restriction is a Tuple expression (or really anything generic). @matthiaskramm can you suggest a workaround?

@jstasiak Unfortunately I am seriously torn about accepting this patch anyways. @JukkaL has measured generic class instantiation vs. regular class instantiation and it is many times slower (1-2 order of magnitude IIRC). Since BaseRequestHandler (or rather a subclass of it) is presumably instantiated for every incoming request. That could slow down request handling quite a bit.

You'd have to do some serious benchmarking to get over this opposition, I'm afraid.

Sorry for wasting your time.

[matthiaskramm]

I believe you can work around the TypeVar issue by saying

TypeVar('foo', Union[x, y])

instead of

TypeVar('foo', x, y)

We can look into supporting the latter, too.

[gvanrossum]

@matthiaskramm Then how does pytype even parse the definition of AnyStr?

[jstasiak]

@gvanrossum I can see slowdown (144ns vs 918ns in a microbenchmark) but would it matter here? My understanding is stub code won't live at runtime anyway and those definitions don't affect the actual socketserver module.

[matthiaskramm]

@gvanrossum: We don't parse stdlib/2.7/typing.pyi or stdlib/2.7/__builtin__.pyi. Those files are both too mypy-specific. We have our own versions.

(I wonder whether the PyCharm people have as well? If yes, we could move those two files into mypy.)

[gvanrossum]

@matthiaskramm A reasonable suggestion, can you file a different issue?

@jstasiak Sure, as long as people just use BaseRequestHandler in type comments it would be fine. But what if I define a subclass to handle only a certain kind of requests? I'd have to write

class MyHandler(BaseRequestHandler[SocketType, Tuple[str, int]]):
    ...

unless I wanted to do everything in stubs, which is not a pleasant way to work (at least not until mypy can somehow internally merge a module with its stub).

I guess the answer is that I simply can't write that class definition outside a stub because the stdlib socketserver.py doesn't define a generic class. But then how would my subclass be type-checked? I worry that there will be some point where my class won't type-check unless I have a way to specify concrete types to be assumed by _RequestT and _AddressT.

[jstasiak]

Fair enough, I didn't think about it. So - back to the original version with client_address and request being unions?

[gvanrossum]

I think not -- I believe the best for now is to leave then Any and add a comment explaining the issue.

[jstasiak]

Just updated

[gvanrossum]

Thanks for understanding!