I don't think this check is appropriate, if someone duck-types BaseHandler, the RequestHandler should accept it restricting the user.

Why would someone duck-type BaseHandler instead of just subclassing it. If there's a good reason then that is fine, otherwise we are just supporting calling this in yet another way, and that way requires us to implement a more complex verification routine.

@remilapeyre I agree with @FFY00, we should make it as simple as we can

I don't think check a class variable inside instance constructor is a good idea. I think it's fine to omit it and use server_handler directly. Those who want to extend should ensure it's of correct type.

It is a class variable, not instance variable. I wonder if the implementation would not be better just making it an instance variable and passing the argument in the constructor.

Because the BaseHandler initial process is dependent on some special variable what is existed in the request. So I don't think to make it as an instance variable is a good idea

I'll fix the docs

Still, I don't see the problem on setting server_handler in the constructor. Am I missing some context here?

If you make it a class attribute, if you only want to change the server handler you still need to subclass WSGIRequestHandler.

        handler = self.server_handler(
            self.rfile, self.wfile, self.get_stderr(), self.get_environ(),
            multithread=False,
        )

the BaseHandler initial process needs the request environment, here's the detail

    def get_environ(self):
        env = self.server.base_environ.copy()
        env['SERVER_PROTOCOL'] = self.request_version
        env['SERVER_SOFTWARE'] = self.server_version
        env['REQUEST_METHOD'] = self.command
        if '?' in self.path:
            path,query = self.path.split('?',1)
        else:
            path,query = self.path,''

        env['PATH_INFO'] = urllib.parse.unquote(path, 'iso-8859-1')
        env['QUERY_STRING'] = query

        host = self.address_string()
        if host != self.client_address[0]:
            env['REMOTE_HOST'] = host
        env['REMOTE_ADDR'] = self.client_address[0]

        if self.headers.get('content-type') is None:
            env['CONTENT_TYPE'] = self.headers.get_content_type()
        else:
            env['CONTENT_TYPE'] = self.headers['content-type']

        length = self.headers.get('content-length')
        if length:
            env['CONTENT_LENGTH'] = length

        for k, v in self.headers.items():
            k=k.replace('-','_').upper(); v=v.strip()
            if k in env:
                continue                    # skip content length, type,etc.
            if 'HTTP_'+k in env:
                env['HTTP_'+k] += ','+v     # comma-separate multiple headers
            else:
                env['HTTP_'+k] = v
        return env

Some of the environment message is only existed in the HTTP request, that means that we need make an instance of BaseHandler for every request income

Sorry, I still fail to see a reason why it wouldn't be better to just have this:

class WSGIRequestHandler(BaseHTTPRequestHandler):
    ...
    def __init__(self, server_handler=ServerHandler, *args, **kwargs):
        self.server_handler = server_handler
        ...

get_environ is an instance method so it never gets called in a context where self.server_handler would not be set.

Oh, I think I get your means, yes, this would be a good idea

@FFY00 Oh No, it doesn't work actually. I have double-checked the code, here's the detail

The WSGIRequestHandler initial process is not controlled by the user, by the server_class

def make_server(
    host, port, app, server_class=WSGIServer, handler_class=WSGIRequestHandler
):
    """Create a new WSGI server listening on `host` and `port` for `app`"""
    server = server_class((host, port), handler_class)
    server.set_app(app)
    return server

The user always uses the ReqeustHandler as a param when they try to use the make_server, and let the server_class to control it when it can be made as an instance. So, if we change the constructor, I think it will make a big and broken change for the make_server function and server_class

That makes sense 😄

Thanks bro/mses, this discussion is very useful for
me!😀😀

I don't know we should expose this attribute or not. For me, the handle() method has already document the behaviour very well.