Mercurial > libervia-backend
changeset 3998:402d31527af4
plugin app manager: `start` doesn't wait anymore for actual app start:
Application may be long to start (e.g. a Docker app may have to download images first,
and even without the downloading, the starting could be long), which may lead to UI
blocking or bridge time out.
To prevent that, `start` is now returning immediately, and 2 new signals are used to
indicate when the application is started, of if something wrong happened.
`start` now returns initial app data, including exposed data without the computed exposed
data. The computed data must be retrieved after the app has been started.
| author | Goffi <goffi@goffi.org> |
|---|---|
| date | Sat, 04 Mar 2023 18:30:47 +0100 |
| parents | 1b7c6ee080b9 |
| children | f7fa3993df28 |
| files | sat/plugins/plugin_app_manager_docker/__init__.py sat/plugins/plugin_misc_app_manager.py |
| diffstat | 2 files changed, 101 insertions(+), 41 deletions(-) [+] |
line wrap: on
line diff
--- a/sat/plugins/plugin_app_manager_docker/__init__.py Sat Mar 04 18:23:33 2023 +0100 +++ b/sat/plugins/plugin_app_manager_docker/__init__.py Sat Mar 04 18:30:47 2023 +0100 @@ -57,7 +57,7 @@ self._am.register(self) async def start(self, app_data: dict) -> None: - await self._am.startCommon(app_data) + await self._am.start_common(app_data) working_dir = app_data['_instance_dir_path'] try: override = app_data['override'] @@ -83,7 +83,7 @@ path=str(working_dir), ) - async def computeExpose(self, app_data: dict) -> dict: + async def compute_expose(self, app_data: dict) -> dict: working_dir = app_data['_instance_dir_path'] expose = app_data['expose'] ports = expose.get('ports', {})
--- a/sat/plugins/plugin_misc_app_manager.py Sat Mar 04 18:23:33 2023 +0100 +++ b/sat/plugins/plugin_misc_app_manager.py Sat Mar 04 18:30:47 2023 +0100 @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -# SàT plugin to manage external applications +# Libervia plugin to manage external applications # Copyright (C) 2009-2021 Jérôme Poisson (goffi@goffi.org) # This program is free software: you can redistribute it and/or modify @@ -17,7 +17,7 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. from pathlib import Path -from typing import Optional, List +from typing import Optional, List, Callable from functools import partial, reduce import tempfile import secrets @@ -95,7 +95,7 @@ "applicationStart", ".plugin", in_sign="ss", - out_sign="", + out_sign="s", method=self._start, async_=True, ) @@ -112,9 +112,19 @@ ".plugin", in_sign="sss", out_sign="s", - method=self._getExposed, + method=self._get_exposed, async_=True, ) + # application has been started succeesfully, + # args: name, instance_id, extra + host.bridge.addSignal( + "application_started", ".plugin", signature="sss" + ) + # application went wrong with the application + # args: name, instance_id, extra + host.bridge.addSignal( + "application_error", ".plugin", signature="sss" + ) yaml.add_constructor( "!sat_conf", self._sat_conf_constr, Loader=Loader) yaml.add_constructor( @@ -133,7 +143,7 @@ data['_instance_dir_obj'].cleanup() def _sat_conf_constr(self, loader, node): - """Get a value from SàT configuration + """Get a value from Libervia configuration A list is expected with either "name" of a config parameter, a one or more of those parameters: @@ -208,7 +218,7 @@ if hasattr(manager, "discover_path"): self.discover(manager.discover_path, manager) - def getManager(self, app_data: dict) -> object: + def get_manager(self, app_data: dict) -> object: """Get manager instance needed for this app @raise exceptions.DataError: something is wrong with the type @@ -231,7 +241,7 @@ raise exceptions.NotFound( f"No manager found to manage app of type {app_type!r}") - def getAppData( + def get_app_data( self, id_type: Optional[str], identifier: str @@ -277,7 +287,7 @@ if manager is None: try: app_data = self.parse(file_path) - manager = self.getManager(app_data) + manager = self.get_manager(app_data) except (exceptions.DataError, exceptions.NotFound) as e: log.warning( f"Can't parse {file_path}, skipping: {e}") @@ -294,7 +304,7 @@ ) def parse(self, file_path: Path, params: Optional[dict] = None) -> dict: - """Parse SàT application file + """Parse Libervia application file @param params: parameters for running this instance @raise exceptions.DataError: something is wrong in the file @@ -341,13 +351,30 @@ def _start(self, app_name, extra): extra = data_format.deserialise(extra) - return defer.ensureDeferred(self.start(str(app_name), extra)) + d = defer.ensureDeferred(self.start(str(app_name), extra)) + d.addCallback(data_format.serialise) + return d async def start( self, app_name: str, extra: Optional[dict] = None, - ) -> None: + ) -> dict: + """Start an application + + @param app_name: name of the application to start + @param extra: extra parameters + @return: data with following keys: + - name (str): canonical application name + - instance (str): instance ID + - started (bool): True if the application is already started + if False, the "application_started" signal should be used to get notificed + when the application is actually started + - expose (dict): exposed data as given by [self.get_exposed] + exposed data which need to be computed are NOT returned, they will + available when the app will be fully started, throught the + [self.get_exposed] method. + """ # FIXME: for now we use the first app manager available for the requested app_name # TODO: implement running multiple instance of the same app if some metadata # to be defined in app_data allows explicitly it. @@ -358,15 +385,28 @@ raise exceptions.NotFound( f"No application found with the name {app_name!r}" ) + log.info(f"starting {app_name!r}") started_data = self._started.setdefault(app_name, []) app_data = self.parse(app_file_path, extra) + app_data["_started"] = False app_data['_file_path'] = app_file_path app_data['_name_canonical'] = app_name single_instance = app_data['single_instance'] + ret_data = { + "name": app_name, + "started": False + } if single_instance: if started_data: - log.info(f"{app_name!r} is already started") - return + instance_data = started_data[0] + instance_id = instance_data["_instance_id"] + ret_data["instance"] = instance_id + ret_data["started"] = instance_data["_started"] + ret_data["expose"] = await self.get_exposed( + instance_id, "instance", {"skip_compute": True} + ) + log.info(f"{app_name!r} is already started or being started") + return ret_data else: cache_path = self.host.memory.getCachePath( PLUGIN_INFO[C.PI_IMPORT_NAME], app_name @@ -377,11 +417,16 @@ dest_dir_obj = tempfile.TemporaryDirectory(prefix="sat_app_") app_data['_instance_dir_obj'] = dest_dir_obj app_data['_instance_dir_path'] = Path(dest_dir_obj.name) - instance_id = app_data['_instance_id'] = shortuuid.uuid() - manager = self.getManager(app_data) + instance_id = ret_data["instance"] = app_data['_instance_id'] = shortuuid.uuid() + manager = self.get_manager(app_data) app_data['_manager'] = manager started_data.append(app_data) self._instances[instance_id] = app_data + # we retrieve exposed data such as url_prefix which can be useful computed exposed + # data must wait for the app to be started, so we skip them for now + ret_data["expose"] = await self.get_exposed( + instance_id, "instance", {"skip_compute": True} + ) try: start = manager.start @@ -390,8 +435,29 @@ f"{manager.name} doesn't have the mandatory \"start\" method" ) else: - await start(app_data) - log.info(f"{app_name!r} started") + defer.ensureDeferred(self.start_app(start, app_data)) + return ret_data + + async def start_app(self, start_cb: Callable, app_data: dict) -> None: + app_name = app_data["_name_canonical"] + instance_id = app_data["_instance_id"] + try: + await start_cb(app_data) + except Exception as e: + log.exception(f"Can't start libervia app {app_name!r}") + self.host.bridge.application_error( + app_name, + instance_id, + data_format.serialise( + { + "class": str(type(e)), + "msg": str(e) + } + )) + else: + app_data["_started"] = True + self.host.bridge.application_started(app_name, instance_id, "") + log.info(f"{app_name!r} started") def _stop(self, identifier, id_type, extra): extra = data_format.deserialise(extra) @@ -407,7 +473,7 @@ if extra is None: extra = {} - app_data = self.getAppData(id_type, identifier) + app_data = self.get_app_data(id_type, identifier) log.info(f"stopping {app_data['name']!r}") @@ -447,21 +513,13 @@ log.info(f"{app_name!r} stopped") - def _getExposed(self, identifier, id_type, extra): + def _get_exposed(self, identifier, id_type, extra): extra = data_format.deserialise(extra) - d = defer.ensureDeferred(self.getExposed(identifier, id_type, extra)) + d = defer.ensureDeferred(self.get_exposed(identifier, id_type, extra)) d.addCallback(lambda d: data_format.serialise(d)) return d - def getValueFromPath(self, app_data: dict, path: List[str]) -> any: - """Retrieve a value set in the data from it path - - @param path: list of key to use in app_data to retrieve the value - @return: found value - @raise NotFound: the value can't be found - """ - - async def getExposed( + async def get_exposed( self, identifier: str, id_type: Optional[str] = None, @@ -469,10 +527,10 @@ ) -> dict: """Get data exposed by the application - The manager's "computeExpose" method will be called if it exists. It can be used + The manager's "compute_expose" method will be called if it exists. It can be used to handle manager specific conventions. """ - app_data = self.getAppData(id_type, identifier) + app_data = self.get_app_data(id_type, identifier) if app_data.get('_exposed_computed', False): return app_data['expose'] if extra is None: @@ -500,17 +558,20 @@ f"Can't retrieve exposed value for url_prefix: {e}") del expose["url_prefix"] + if extra.get("skip_compute", False): + return expose + try: - computeExpose = app_data['_manager'].computeExpose + compute_expose = app_data['_manager'].compute_expose except AttributeError: pass else: - await computeExpose(app_data) + await compute_expose(app_data) app_data['_exposed_computed'] = True return expose - async def _doPrepare( + async def _do_prepare( self, app_data: dict, ) -> None: @@ -548,7 +609,7 @@ if prepare: raise exceptions.InternalError('"prepare" should be empty') - async def _doCreateFiles( + async def _do_create_files( self, app_data: dict, ) -> None: @@ -566,11 +627,10 @@ f.write(data.get("content", "")) log.debug(f"{path} created") - async def startCommon(self, app_data: dict) -> None: + async def start_common(self, app_data: dict) -> None: """Method running common action when starting a manager It should be called by managers in "start" method. """ - log.info(f"starting {app_data['name']!r}") - await self._doPrepare(app_data) - await self._doCreateFiles(app_data) + await self._do_prepare(app_data) + await self._do_create_files(app_data)