From 5d8e35653fca0d7ee01519d556c7851089de82ea Mon Sep 17 00:00:00 2001 From: David Lord Date: Mon, 1 Aug 2022 09:13:01 -0700 Subject: [PATCH] refactor lazy loading Remove the `--eager-loading/--lazy-loading` options and the `DispatchingApp` middleware. The `run` command handles loading exceptions directly. The reloader always prints out tracebacks immediately and always defers raising the error. --- CHANGES.rst | 6 +++ docs/quickstart.rst | 4 +- docs/server.rst | 9 +--- src/flask/app.py | 2 +- src/flask/cli.py | 106 +++++++++----------------------------------- tests/test_cli.py | 22 --------- 6 files changed, 31 insertions(+), 118 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d7898652..f8ebc729 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -92,6 +92,12 @@ Unreleased JSON response like a dict is. :issue:`4672` - When type checking, allow ``TypedDict`` to be returned from view functions. :pr:`4695` +- Remove the ``--eager-loading/--lazy-loading`` options from the + ``flask run`` command. The app is always eager loaded the first + time, then lazily loaded in the reloader. The reloader always prints + errors immediately but continues serving. Remove the internal + ``DispatchingApp`` middleware used by the previous implementation. + :issue:`4715` Version 2.1.3 diff --git a/docs/quickstart.rst b/docs/quickstart.rst index ed5f4557..a824f6f3 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -46,7 +46,7 @@ is with the ``-app`` option. .. code-block:: text $ flask --app hello run - * Serving Flask app 'hello' (lazy loading) + * Serving Flask app 'hello' * Running on http://127.0.0.1:5000 (Press CTRL+C to quit) .. admonition:: Application Discovery Behavior @@ -110,7 +110,7 @@ To enable all development features, set the ``--env`` option to .. code-block:: text $ flask --app hello --env development run - * Serving Flask app 'hello' (lazy loading) + * Serving Flask app 'hello' * Environment: development * Debug mode: on * Running on http://127.0.0.1:5000 (Press CTRL+C to quit) diff --git a/docs/server.rst b/docs/server.rst index aa1438a3..5c78f171 100644 --- a/docs/server.rst +++ b/docs/server.rst @@ -127,24 +127,19 @@ macOS Monterey and later automatically starts a service that uses port disable "AirPlay Receiver". -Lazy or Eager Loading -~~~~~~~~~~~~~~~~~~~~~ +Deferred Errors on Reload +~~~~~~~~~~~~~~~~~~~~~~~~~ When using the ``flask run`` command with the reloader, the server will continue to run even if you introduce syntax errors or other initialization errors into the code. Accessing the site will show the interactive debugger for the error, rather than crashing the server. -This feature is called "lazy loading". If a syntax error is already present when calling ``flask run``, it will fail immediately and show the traceback rather than waiting until the site is accessed. This is intended to make errors more visible initially while still allowing the server to handle errors on reload. -To override this behavior and always fail immediately, even on reload, -pass the ``--eager-loading`` option. To always keep the server running, -even on the initial call, pass ``--lazy-loading``. - In Code ------- diff --git a/src/flask/app.py b/src/flask/app.py index da7bd32d..856c0d9d 100644 --- a/src/flask/app.py +++ b/src/flask/app.py @@ -998,7 +998,7 @@ class Flask(Scaffold): options.setdefault("use_debugger", self.debug) options.setdefault("threaded", True) - cli.show_server_banner(self.env, self.debug, self.name, False) + cli.show_server_banner(self.env, self.debug, self.name) from werkzeug.serving import run_simple diff --git a/src/flask/cli.py b/src/flask/cli.py index ba0db467..e75dff31 100644 --- a/src/flask/cli.py +++ b/src/flask/cli.py @@ -10,8 +10,6 @@ import traceback import typing as t from functools import update_wrapper from operator import attrgetter -from threading import Lock -from threading import Thread import click from click.core import ParameterSource @@ -267,74 +265,6 @@ version_option = click.Option( ) -class DispatchingApp: - """Special application that dispatches to a Flask application which - is imported by name in a background thread. If an error happens - it is recorded and shown as part of the WSGI handling which in case - of the Werkzeug debugger means that it shows up in the browser. - """ - - def __init__(self, loader, use_eager_loading=None): - self.loader = loader - self._app = None - self._lock = Lock() - self._bg_loading_exc = None - - if use_eager_loading is None: - use_eager_loading = not is_running_from_reloader() - - if use_eager_loading: - self._load_unlocked() - else: - self._load_in_background() - - def _load_in_background(self): - # Store the Click context and push it in the loader thread so - # script_info is still available. - ctx = click.get_current_context(silent=True) - - def _load_app(): - __traceback_hide__ = True # noqa: F841 - - with self._lock: - if ctx is not None: - click.globals.push_context(ctx) - - try: - self._load_unlocked() - except Exception as e: - self._bg_loading_exc = e - - t = Thread(target=_load_app, args=()) - t.start() - - def _flush_bg_loading_exception(self): - __traceback_hide__ = True # noqa: F841 - exc = self._bg_loading_exc - - if exc is not None: - self._bg_loading_exc = None - raise exc - - def _load_unlocked(self): - __traceback_hide__ = True # noqa: F841 - self._app = rv = self.loader() - self._bg_loading_exc = None - return rv - - def __call__(self, environ, start_response): - __traceback_hide__ = True # noqa: F841 - if self._app is not None: - return self._app(environ, start_response) - self._flush_bg_loading_exception() - with self._lock: - if self._app is not None: - rv = self._app - else: - rv = self._load_unlocked() - return rv(environ, start_response) - - class ScriptInfo: """Helper object to deal with Flask applications. This is usually not necessary to interface with as it's used internally in the dispatching @@ -811,7 +741,7 @@ def load_dotenv(path: str | os.PathLike | None = None) -> bool: return loaded # True if at least one file was located and loaded. -def show_server_banner(env, debug, app_import_path, eager_loading): +def show_server_banner(env, debug, app_import_path): """Show extra startup messages the first time the server is run, ignoring the reloader. """ @@ -819,12 +749,7 @@ def show_server_banner(env, debug, app_import_path, eager_loading): return if app_import_path is not None: - message = f" * Serving Flask app {app_import_path!r}" - - if not eager_loading: - message += " (lazy loading)" - - click.echo(message) + click.echo(f" * Serving Flask app '{app_import_path}'") click.echo(f" * Environment: {env}") @@ -963,12 +888,6 @@ class SeparatedPathType(click.Path): help="Enable or disable the debugger. By default the debugger " "is active if debug is enabled.", ) -@click.option( - "--eager-loading/--lazy-loading", - default=None, - help="Enable or disable eager loading. By default eager " - "loading is enabled if the reloader is disabled.", -) @click.option( "--with-threads/--without-threads", default=True, @@ -1000,7 +919,6 @@ def run_command( port, reload, debugger, - eager_loading, with_threads, cert, extra_files, @@ -1014,7 +932,23 @@ def run_command( The reloader and debugger are enabled by default with the '--env development' or '--debug' options. """ - app = DispatchingApp(info.load_app, use_eager_loading=eager_loading) + try: + app = info.load_app() + except Exception as e: + if is_running_from_reloader(): + # When reloading, print out the error immediately, but raise + # it later so the debugger or server can handle it. + traceback.print_exc() + err = e + + def app(environ, start_response): + raise err from None + + else: + # When not reloading, raise the error immediately so the + # command fails. + raise e from None + debug = get_debug_flag() if reload is None: @@ -1023,7 +957,7 @@ def run_command( if debugger is None: debugger = debug - show_server_banner(get_env(), debug, info.app_import_path, eager_loading) + show_server_banner(get_env(), debug, info.app_import_path) from werkzeug.serving import run_simple diff --git a/tests/test_cli.py b/tests/test_cli.py index 107cbb11..0d9625b1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,7 +1,6 @@ # This file was part of Flask-CLI and was modified under the terms of # its Revised BSD License. Copyright © 2015 CERN. import os -import platform import ssl import sys import types @@ -17,7 +16,6 @@ from flask import Blueprint from flask import current_app from flask import Flask from flask.cli import AppGroup -from flask.cli import DispatchingApp from flask.cli import find_best_app from flask.cli import FlaskGroup from flask.cli import get_version @@ -290,26 +288,6 @@ def test_scriptinfo(test_apps, monkeypatch): assert app.name == "testapp" -@pytest.mark.xfail(platform.python_implementation() == "PyPy", reason="flaky on pypy") -def test_lazy_load_error(monkeypatch): - """When using lazy loading, the correct exception should be - re-raised. - """ - - class BadExc(Exception): - pass - - def bad_load(): - raise BadExc - - lazy = DispatchingApp(bad_load, use_eager_loading=False) - - # reduce flakiness by waiting for the internal loading lock - with lazy._lock: - with pytest.raises(BadExc): - lazy._flush_bg_loading_exception() - - def test_app_cli_has_app_context(app, runner): def _param_cb(ctx, param, value): # current_app should be available in parameter callbacks