Deprecate dev server shutdown function

[davidism]

This was added in #36 as a way to shutdown local applications, environ["werkzeug.server.shutdown"](). However, the advice for quite some time has been to use a production server even when running local applications; the development server is only for development.

The issue and code comments both describe the implementation as "horrible". The implementation of it is in fact pretty weird, having to touch a double underscore "really private" variable, as well as jump through hoops to work with the HTTP server loop and handle reloader processes.

Additionally, users see this and then are confused why they can only shut down their application when using the development server. Or they want to be able to do more process management such as reloads, which is definitely out of scope.

Is the shutdown function really serving a useful purpose at this point? If we remove it, maybe it can be replaced with a page in the deploy docs about running a production server in a subprocess.

[mitsuhiko]

Is the shutdown function really serving a useful purpose at this point? If we remove it, maybe it can be replaced with a page in the deploy docs about running a production server in a subprocess.

I cannot say if it's really useful but it was really useful when it was added originally and I am in fact still using it for a small tool where I spawn a local server in my CLI for accepting an oauth redirect flow. After the browser navigates to my CLI's temporary server I shut down the listener. In that case I would likely not replace it with a production server because that really doesn't belong into that type of app.

There are also lots of people who use it when using the werkzeug server for testing. You can validate the uses with a quick github code search: https://github.com/search?q=%22werkzeug.server.shutdown%22&type=Code

[davidism]

I'm not suggesting that there are not uses of it, I'm suggesting that it is widely misunderstood and sets incorrect expectations.

I was aware of the code search results, but they mostly seem to be two categories: shutting down a live server test, which is probably better handled by pytest-xprocess, mutiprocessing.Process.terminate(), or equivalent; or shutting down a real live server, which is the misuse of the dev server that we're trying to prevent.

I'll admit, waiting then retrieving a value from a subprocess (either with the dev server or a production server), as opposed to running a single test request, requires a bit more setup to use a multiprocessing.Queue to wait for the value. Your use case makes sense, but it seems to be the tiny minority compared to how it's mostly being used.

[davidism]

I think a basic example of a test that uses Process() and terminate(), as well as this example of retrieving a value, would be appropriate documentation in place of the shutdown() function.

import multiprocessing

from werkzeug import Request
from werkzeug import Response
from werkzeug import run_simple


def get_token(q: multiprocessing.Queue):
    @Request.application
    def app(request):
        q.put(request.args["token"])
        return Response("", 204)

    run_simple("localhost", 5000, app)


if __name__ == "__main__":
    q = multiprocessing.Queue()
    p = multiprocessing.Process(target=get_token, args=(q,))
    p.start()
    print("waiting")
    token = q.get(block=True)
    p.terminate()
    print(token)

[davidism]

I'll also point out that Python's HTTPServer already has a shutdown() function, but it deadlocks when called from the same thread. If you get the server instance first, then pass its serve_forever method to a thread (instead of a process), you could call server.shutdown() from the main thread then join() the server thread. You can't do this with run_simple since it starts serving immediately, but we also provide the much less well known make_server function. Here's the same example as above, but using a thread instead of a process:

import threading
from queue import Queue

from werkzeug import Request
from werkzeug import Response
from werkzeug.serving import make_server


@Request.application
def app(request):
    q.put(request.args["token"])
    return Response("", 204)


q = Queue()
s = make_server("localhost", 5000, app)
t = threading.Thread(target=s.serve_forever)
t.start()
print("waiting")
token = q.get(block=True)
s.shutdown()
t.join()
print(token)

[mitsuhiko]

I'm not suggesting that there are not uses of it, I'm suggesting that it is widely misunderstood and sets incorrect expectations.

For what it's worth I think this is okay. Just because a small number of users might be have bad expectations, does not mean everybody else cannot benefit from having a feature.

The workarounds for not having a way to shut down the server are all pretty awful, particularly the workarounds with going via multiprocessing or other solutions which are even worse from an implementation POV. I think ultimately there will just be a lot of worse solutions and worse user experience as a result to this issue (particularly with people using the server for testing) than before. I don't think the feature is broken for the people that use it currently.

[davidism]

Just realized that when testing, you already have to be running the server in a thread or subprocess, otherwise the server just blocks so no test code would be run. So having werkzeug.server.shutdown() is saving minimal work, they would already be using most of the code above (or just using pytest-xprocess).

Outside testing, the vast majority of search results are about shutting down the server in general, in what seems to be production systems, and then people continually being told that it only works for the dev server which they shouldn't be running in production anyway. I'd prefer to remove this foot-gun.

I don't think the solutions are particularly awful, beyond being more verbose, but there's probably a better way to write it than what I came up with quickly. It has the benefit of being agnostic to the server being used, and demonstrates how to run a server without blocking, which is another fairly common question. Overall, I'm still leaning towards removing this.