Thanks for the extra effort on this to satisfy multiple people's opinions. It's never an easy thing, especially when dealing with a decentralized group. My following comments are largely based on the public API. I haven't done a full review of the changes made to the tests.


All extraneous spaces should be removed. For example the first line here:

+        
+        redirect_path = self._redirect_path(path)
+        if redirect_path:


> Lists the files we want the server to check and render to client. We can add/remove file names as required. Order of files denotes priority. Defaults to ['index.htm', 'index.html']

I think this might read a little better as:

"Overridable index files. If any of these files are present in a request pointing to a directory on the server's file system, the contents of the file will be returned. Otherwise, a directory listing will be returned. Defaults to ['index.htm', 'index.html']. Order of files denotes priority.

That said, I'm not a wordsmith so there may be a (much) better way of structuring this.

> +class HTTPStatusType(Enum):

This doesn't sit quite well with me and I think it really should be removed. There are very few cases (if any) that I can think of where this would be useful. Cases where I've encountered the need for a range check generally is around subsets (i.e. 200 and 201 mean entirely different things and /shouldn't/ be globbed together). Additionally (this is subjective), I find it a little strange that the value of an enum is a range.

If for some reason it stays, I think the values should at least be sets allowing for O(1) lookup.

> +    def redirect(self, url, status=HTTPStatus.MOVED_PERMANENTLY):

There should be an assertion in this block that the status is one of the redirect codes (301, 302, 308). Calling redirect('/foo', status=200) doesn't make sense.

>+         if os.path.isfile(path):
 +            return self.get_file(path)
 +
 +        # `path` has file with filename in self.index_files
 +        for index_file in self.index_files:
 +            index = os.path.join(path, index_file)

Before the directory-related logic executes, you could add a check in for isdir() and short circuit with a 404 rather than relying on an index file not being found and then running into list_directory(), which would result in a 404 with the message "No permission to list directory", which isn't actually correct in the case where the directory doesn't exist.

> get_status_type, apply_success_headers, apply_headers

I think that these three methods should be removed as they're superfluous, make the code less readable and is inconsistent with how responses and headers are sent throughout the rest of server.py. The general practice is:

self.send_response()
self.send_header()
self.send_header()
self.end_headers()

The code in this patch should match that.

get_status_type is not needed as it /should/, if anything, be a single line passthrough to a reverse lookup mapping. Iterating over each enum followed by an iteration over each value is not optimal behaviour (O(N) rather than O(1)).