From a3de7f63a0f3fa5932ed556ca30b57e2ba9d74ac Mon Sep 17 00:00:00 2001 From: Jakob Pinterits Date: Thu, 24 Oct 2024 22:03:53 +0200 Subject: [PATCH] sanified app loading --- rio/app_server/fastapi_server.py | 6 +-- rio/cli/run_project/app_loading.py | 22 +++++---- rio/cli/run_project/arbiter.py | 39 ++++++++------- rio/cli/run_project/uvicorn_worker.py | 71 ++++++++------------------- 4 files changed, 55 insertions(+), 83 deletions(-) diff --git a/rio/app_server/fastapi_server.py b/rio/app_server/fastapi_server.py index 1a82d1e6..b0cb30d6 100644 --- a/rio/app_server/fastapi_server.py +++ b/rio/app_server/fastapi_server.py @@ -201,8 +201,6 @@ class FastapiServer(fastapi.FastAPI, AbstractAppServer): base_url=base_url, ) - self.internal_on_app_start = internal_on_app_start - # If a URL was provided, run some sanity checks if base_url is not None: # If the URL is missing a protocol, yarl doesn't consider it @@ -228,8 +226,8 @@ class FastapiServer(fastapi.FastAPI, AbstractAppServer): "The app's base URL cannot contain a fragment." ) - # All good - self.base_url = base_url + self.internal_on_app_start = internal_on_app_start + self.base_url = base_url # While this Event is unset, no new Sessions can be created. This is # used to ensure that no clients (re-)connect while `rio run` is diff --git a/rio/cli/run_project/app_loading.py b/rio/cli/run_project/app_loading.py index ef9aca9c..5b79d33a 100644 --- a/rio/cli/run_project/app_loading.py +++ b/rio/cli/run_project/app_loading.py @@ -11,8 +11,8 @@ import path_imports import revel import rio -import rio.global_state import rio.app_server.fastapi_server +import rio.global_state from rio import icon_registry from ... import project_config @@ -211,13 +211,15 @@ def import_app_module( def load_user_app( proj: project_config.RioProjectConfig, -) -> tuple[ - rio.App, - rio.app_server.fastapi_server.FastapiServer | None, -]: +) -> rio.app_server.fastapi_server.FastapiServer: """ - Load and return the user app. Raises `AppLoadError` if the app can't be + Load and return the user's app. Raises `AppLoadError` if the app can't be loaded for whichever reason. + + The result is actually an app server, rather than just the app. This is done + so the user can use `as_fastapi` on a Rio app and still use `rio run` to run + that app. If you actually need the app object, it's stored inside the app + server. """ # Import the app module try: @@ -268,12 +270,14 @@ def load_user_app( if len(as_fastapi_apps) > 0: app_list = as_fastapi_apps app_server = as_fastapi_apps[0][1] - app_instance = app_server.app # Case: Rio app elif len(rio_apps) > 0: app_list = rio_apps - app_server = None app_instance = rio_apps[0][1] + app_server = app_instance.as_fastapi() + assert isinstance( + app_server, rio.app_server.fastapi_server.FastapiServer + ) # Case: No app else: raise AppLoadError( @@ -291,4 +295,4 @@ def load_user_app( f"{main_file_reference} defines multiple Rio apps: {variables_string}. Please make sure there is exactly one." ) - return app_instance, app_server + return app_server diff --git a/rio/cli/run_project/arbiter.py b/rio/cli/run_project/arbiter.py index 370e07b0..c568f21f 100644 --- a/rio/cli/run_project/arbiter.py +++ b/rio/cli/run_project/arbiter.py @@ -292,16 +292,16 @@ class Arbiter: def try_load_app( self, ) -> tuple[ - rio.App, - rio.app_server.fastapi_server.FastapiServer | None, + rio.app_server.fastapi_server.FastapiServer, Exception | None, ]: """ Tries to load the user's app. If it fails, a dummy app is created and - returned, unless running in release mode. + returned, unless running in release mode. (In release mode screams and + exits the entire process.) - Returns the app instance, the app's server instance and an exception if - the app could not be loaded. + Returns the app server instance and an exception if the app could not be + loaded. The app server is returned in case the user has called `as_fastapi` on their app instance. In that case the actual fastapi app should be @@ -310,7 +310,7 @@ class Arbiter: rio.cli._logger.debug("Trying to load the app") try: - app, app_server = app_loading.load_user_app(self.proj) + app_server = app_loading.load_user_app(self.proj) except app_loading.AppLoadError as err: if err.__cause__ is not None: @@ -327,21 +327,23 @@ class Arbiter: # Otherwise create a placeholder app which displays the error # message - return ( - app_loading.make_error_message_app( - err, - self.proj.project_directory, - self._app_theme, - ), - None, + app = app_loading.make_error_message_app( err, + self.proj.project_directory, + self._app_theme, ) + app_server = app.as_fastapi() + assert isinstance( + app_server, rio.app_server.fastapi_server.FastapiServer + ) + + return app_server, err # Remember the app's theme. If in the future a placeholder app is used, # this theme will be used for it. - self._app_theme = app._theme + self._app_theme = app_server.app._theme - return app, app_server, None + return app_server, None def run(self) -> None: assert not self._stop_requested.is_set() @@ -517,7 +519,7 @@ class Arbiter: apply_monkeypatches() # Try to load the app - app, app_server, _ = self.try_load_app() + app_server, _ = self.try_load_app() # Start the file watcher if self.debug_mode: @@ -539,7 +541,6 @@ class Arbiter: self._uvicorn_worker = uvicorn_worker.UvicornWorker( push_event=self.push_event, - app=app, app_server=app_server, socket=sock, quiet=self.quiet, @@ -744,10 +745,10 @@ window.setConnectionLostPopupVisible(true); await app_server._call_on_app_close() # Load the user's app again - new_app, new_app_server, loading_error = self.try_load_app() + new_app_server, loading_error = self.try_load_app() # Replace the app which is currently hosted by uvicorn - self._uvicorn_worker.replace_app(new_app, new_app_server) + self._uvicorn_worker.replace_app(new_app_server) # The app has changed, but the uvicorn server is still the same. # Because of this, uvicorn won't call the `on_app_start` function - diff --git a/rio/cli/run_project/uvicorn_worker.py b/rio/cli/run_project/uvicorn_worker.py index d55eaf3d..43ee3024 100644 --- a/rio/cli/run_project/uvicorn_worker.py +++ b/rio/cli/run_project/uvicorn_worker.py @@ -21,8 +21,7 @@ class UvicornWorker: self, *, push_event: t.Callable[[run_models.Event], None], - app: rio.App, - app_server: rio.app_server.fastapi_server.FastapiServer | None, + app_server: rio.app_server.fastapi_server.FastapiServer, socket: socket.socket, quiet: bool, debug_mode: bool, @@ -40,38 +39,15 @@ class UvicornWorker: # The app server used to host the app. # - # This can optionally be provided to the constructor. If not, it will be - # created when the worker is started. This allows for the app to be - # either a Rio app or a FastAPI app (derived from a Rio app). - self.app = app - self.app_server: rio.app_server.fastapi_server.FastapiServer | None = ( - None - ) - - self.replace_app(app, app_server) - - def _create_and_store_app_server(self) -> None: - app_server = self.app._as_fastapi( - debug_mode=self.debug_mode, - running_in_window=self.run_in_window, - internal_on_app_start=lambda: self.on_server_is_ready_or_failed.set_result( - None - ), - base_url=self.base_url, - ) - assert isinstance( - app_server, rio.app_server.fastapi_server.FastapiServer - ) + # While already provided in the constructor, this needs some values to + # be overridden. `replace_app` already handles this, so delegate to that + # function. self.app_server = app_server + self.replace_app(app_server) async def run(self) -> None: rio.cli._logger.debug("Uvicorn worker is starting") - # Create the app server - if self.app_server is None: - self._create_and_store_app_server() - assert self.app_server is not None - # Instead of using the ASGI app directly, create a transparent shim that # redirect's to the worker's currently stored app server. This allows # replacing the app server at will because the shim always remains the @@ -153,36 +129,29 @@ class UvicornWorker: def replace_app( self, - app: rio.App, - app_server: rio.app_server.fastapi_server.FastapiServer | None, + app_server: rio.app_server.fastapi_server.FastapiServer, ) -> None: """ Replace the app currently running in the server with a new one. The worker must already be running for this to work. """ + assert ( + app_server.internal_on_app_start is None + ), app_server.internal_on_app_start + # Store the new app - self.app = app + self.app_server = app_server - # And create a new app server. This is necessary, because the mounted - # sub-apps may have changed. This ensures they're up to date. - if app_server is None: - self._create_and_store_app_server() + self.app_server.debug_mode = self.debug_mode + self.app_server.running_in_window = self.run_in_window + self.app_server.internal_on_app_start = ( + lambda: self.on_server_is_ready_or_failed.set_result(None) + ) + + if self.base_url is None: + self.app_server.base_url = None else: - self.app_server = app_server - - self.app_server.debug_mode = self.debug_mode - self.app_server.running_in_window = self.run_in_window - self.app_server.internal_on_app_start = ( - lambda: self.on_server_is_ready_or_failed.set_result(None) - ) - - if self.base_url is None: - self.app_server.base_url = None - else: - self.app_server.base_url = utils.normalize_url(self.base_url) - - assert self.app_server is not None - assert self.app_server.app is self.app + self.app_server.base_url = utils.normalize_url(self.base_url) # There is no need to inject the new app or server anywhere. Since # uvicorn was fed a shim function instead of the app directly, any