From: Marcel van der Veldt Date: Fri, 17 Mar 2023 14:35:21 +0000 (+0100) Subject: Fix configuration flows (#545) X-Git-Url: https://git.kitaultman.com/?a=commitdiff_plain;h=2b333b89cea0eb6a7ed4177b67b1e75e8b34a433;p=music-assistant-server.git Fix configuration flows (#545) - Validate config before saving - Ensure all players and providers have a default config - Various config related fixes - Improve add provider flow --- diff --git a/music_assistant/common/models/config_entries.py b/music_assistant/common/models/config_entries.py index b5e9f543..433d14fa 100644 --- a/music_assistant/common/models/config_entries.py +++ b/music_assistant/common/models/config_entries.py @@ -206,6 +206,13 @@ class Config(DataClassDictMixin): return changed_keys + def validate(self) -> None: + """Validate if configuration is valid.""" + # For now we just use the parse method to check for not allowed None values + # this can be extended later + for value in self.values.values(): + value.parse(value, value.value, allow_none=False) + @dataclass class ProviderConfig(Config): @@ -218,6 +225,8 @@ class ProviderConfig(Config): enabled: bool = True # name: an (optional) custom name for this provider instance/config name: str | None = None + # last_error: an optional error message if the provider could not be setup with this config + last_error: str | None = None @dataclass @@ -230,6 +239,10 @@ class PlayerConfig(Config): enabled: bool = True # name: an (optional) custom name for this player name: str | None = None + # available: boolean to indicate if the player is available + available: bool = True + # default_name: default name to use when there is name available + default_name: str | None = None @dataclass diff --git a/music_assistant/common/models/provider.py b/music_assistant/common/models/provider.py index 80ab604f..58a166ed 100644 --- a/music_assistant/common/models/provider.py +++ b/music_assistant/common/models/provider.py @@ -56,7 +56,6 @@ class ProviderInstance(TypedDict): instance_id: str supported_features: list[ProviderFeature] available: bool - last_error: str | None @dataclass diff --git a/music_assistant/server/controllers/config.py b/music_assistant/server/controllers/config.py index f74b6674..83c7ab13 100644 --- a/music_assistant/server/controllers/config.py +++ b/music_assistant/server/controllers/config.py @@ -177,71 +177,40 @@ class ConfigController: raise KeyError(f"No config found for provider id {instance_id}") @api_command("config/providers/update") - def update_provider_config(self, instance_id: str, update: ConfigUpdate) -> None: + async def update_provider_config(self, instance_id: str, update: ConfigUpdate) -> None: """Update ProviderConfig.""" config = self.get_provider_config(instance_id) changed_keys = config.update(update) - - if not changed_keys: + try: + prov = self.mass.get_provider(instance_id) + available = prov.available + except ProviderUnavailableError: + available = False + if not changed_keys and (config.enabled == available): # no changes return - + # try to load the provider first to catch errors before we save it. + if config.enabled: + await self.mass.load_provider(config) + # load succeeded, save new config conf_key = f"{CONF_PROVIDERS}/{instance_id}" self.set(conf_key, config.to_raw()) - updated_config = self.get_provider_config(config.instance_id) - self.mass.create_task(self.mass.load_provider(updated_config)) - @api_command("config/providers/create") - def create_provider_config( - self, provider_domain: str, default_enabled: bool = False + @api_command("config/providers/add") + async def add_provider_config( + self, provider_domain: str, config: ProviderConfig | None = None ) -> ProviderConfig: - """Create default/empty ProviderConfig. - - This is intended to be used as helper method to add a new provider, - and it performs some quick sanity checks as well as handling the - instance_id generation. - """ - # lookup provider manifest - for prov in self.mass.get_available_providers(): - if prov.domain == provider_domain: - manifest = prov - break - else: - raise KeyError(f"Unknown provider domain: {provider_domain}") - - # determine instance id based on previous configs - existing = { - x.instance_id for x in self.get_provider_configs(provider_domain=provider_domain) - } - - if existing and not manifest.multi_instance: - raise ValueError(f"Provider {manifest.name} does not support multiple instances") - - if len(existing) == 0: - instance_id = provider_domain - name = manifest.name - else: - instance_id = f"{provider_domain}{len(existing)+1}" - name = f"{manifest.name} {len(existing)+1}" - - # all checks passed, return a default config - default_config = ProviderConfig.parse( - prov.config_entries, - { - "type": manifest.type.value, - "domain": manifest.domain, - "instance_id": instance_id, - "name": name, - "enabled": default_enabled, - "values": {}, - }, - ) - - # config provided and checks passed, storeconfig - conf_key = f"{CONF_PROVIDERS}/{instance_id}" - self.set(conf_key, default_config.to_raw()) - - return default_config + """Add new Provider (instance) Config Flow.""" + if not config: + return self._get_default_provider_config(provider_domain) + # if provider config is provided, the frontend wants to submit a new provider instance + # based on the earlier created template config. + # try to load the provider first to catch errors before we save it. + await self.mass.load_provider(config) + # config provided and load success, storeconfig + conf_key = f"{CONF_PROVIDERS}/{config.instance_id}" + self.set(conf_key, config.to_raw()) + return config @api_command("config/providers/remove") async def remove_provider_config(self, instance_id: str) -> None: @@ -256,58 +225,38 @@ class ConfigController: # cleanup entries in library await self.mass.music.cleanup_provider(instance_id) + @api_command("config/providers/reload") + async def reload_provider(self, instance_id: str) -> None: + """Reload provider.""" + config = self.get_provider_config(instance_id) + await self.mass.load_provider(config) + @api_command("config/players") def get_player_configs(self, provider: str | None = None) -> list[PlayerConfig]: """Return all known player configurations, optionally filtered by provider domain.""" - result: dict[str, PlayerConfig] = {} - # we use an intermediate dict to cover both edge cases: - # - player does not yet have a config stored persistently - # - player is disabled in config and not available - - # do all existing players first - for player in self.mass.players: - if provider is not None and player.provider != provider: - continue - result[player.player_id] = self.get_player_config(player.player_id) - - # add remaining configs that do have a config stored but are not (yet) available now - raw_configs = self.get(CONF_PLAYERS, {}) - for player_id, raw_conf in raw_configs.items(): - if player_id in result: - continue - if provider is not None and raw_conf["provider"] != provider: - continue + return [ + self.get_player_config(player_id) + for player_id, raw_conf in self.get(CONF_PLAYERS).items() + if (provider in (None, raw_conf["provider"])) + ] + + @api_command("config/players/get") + def get_player_config(self, player_id: str) -> PlayerConfig: + """Return configuration for a single player.""" + if raw_conf := self.get(f"{CONF_PLAYERS}/{player_id}"): try: prov = self.mass.get_provider(raw_conf["provider"]) prov_entries = prov.get_player_config_entries(player_id) except (ProviderUnavailableError, PlayerUnavailableError): prov_entries = tuple() + raw_conf["available"] = False + raw_conf["name"] = ( + raw_conf.get("name") or raw_conf.get("default_name") or raw_conf["player_id"] + ) entries = DEFAULT_PLAYER_CONFIG_ENTRIES + prov_entries - result[player.player_id] = PlayerConfig.parse(entries, raw_conf) - - return list(result.values()) - - @api_command("config/players/get") - def get_player_config(self, player_id: str) -> PlayerConfig: - """Return configuration for a single player.""" - conf = self.get(f"{CONF_PLAYERS}/{player_id}") - if not conf: - player = self.mass.players.get(player_id, raise_unavailable=False) - conf = { - "provider": player.provider, - "player_id": player_id, - "enabled": player.enabled_by_default, - } - - try: - prov = self.mass.get_provider(conf["provider"]) - prov_entries = prov.get_player_config_entries(player_id) - except (ProviderUnavailableError, PlayerUnavailableError): - prov_entries = tuple() - - entries = DEFAULT_PLAYER_CONFIG_ENTRIES + prov_entries - return PlayerConfig.parse(entries, conf) + return PlayerConfig.parse(entries, raw_conf) + raise KeyError(f"No config found for player id {player_id}") @api_command("config/players/get_value") def get_player_config_value(self, player_id: str, key: str) -> ConfigEntryValue: @@ -348,10 +297,6 @@ class ConfigController: player = self.mass.players.get(config.player_id) player.enabled = config.enabled self.mass.players.update(config.player_id) - # copy playername to find back the playername if its disabled - if not config.enabled and not config.name: - config.name = player.display_name - self.set(conf_key, config.to_raw()) except PlayerUnavailableError: pass @@ -383,6 +328,86 @@ class ConfigController: assert isinstance(provider, PlayerProvider) provider.on_player_config_removed(player_id) + def create_default_player_config(self, player_id: str, provider: str, name: str) -> None: + """ + Create default/empty PlayerConfig. + + This is meant as helper to create default configs when a player is registered. + Called by the player manager on player register. + """ + # return early if the config already exists + if self.get(f"{CONF_PLAYERS}/{player_id}"): + # update default name if needed + self.set(f"{CONF_PLAYERS}/{player_id}/default_name", name) + return + # config does not yet exist, create a default one + conf_key = f"{CONF_PLAYERS}/{player_id}" + default_conf = PlayerConfig( + values={}, provider=provider, player_id=player_id, default_name=name + ) + self.set( + conf_key, + default_conf.to_raw(), + ) + + def create_default_provider_config(self, provider_domain: str) -> None: + """ + Create default ProviderConfig. + + This is meant as helper to create default configs for default enabled providers. + Called by the server initialization code which load all providers at startup. + """ + for conf in self.get_provider_configs(provider_domain=provider_domain): + # return if there is already a config + return + # config does not yet exist, create a default one + default_config = self._get_default_provider_config(provider_domain) + conf_key = f"{CONF_PROVIDERS}/{default_config.instance_id}" + self.set(conf_key, default_config.to_raw()) + + def _get_default_provider_config(self, provider_domain: str) -> ProviderConfig: + """ + Return default/empty ProviderConfig. + + This is intended to be used as helper method to add a new provider, + and it performs some quick sanity checks as well as handling the + instance_id generation. + """ + # lookup provider manifest + for prov in self.mass.get_available_providers(): + if prov.domain == provider_domain: + manifest = prov + break + else: + raise KeyError(f"Unknown provider domain: {provider_domain}") + + # determine instance id based on previous configs + existing = { + x.instance_id for x in self.get_provider_configs(provider_domain=provider_domain) + } + + if existing and not manifest.multi_instance: + raise ValueError(f"Provider {manifest.name} does not support multiple instances") + + if len(existing) == 0: + instance_id = provider_domain + name = manifest.name + else: + instance_id = f"{provider_domain}{len(existing)+1}" + name = f"{manifest.name} {len(existing)+1}" + + # all checks passed, return a default config + return ProviderConfig.parse( + prov.config_entries, + { + "type": manifest.type.value, + "domain": manifest.domain, + "instance_id": instance_id, + "name": name, + "values": {}, + }, + ) + async def _load(self) -> None: """Load data from persistent storage.""" assert not self._data, "Already loaded" diff --git a/music_assistant/server/controllers/players.py b/music_assistant/server/controllers/players.py index 7ab6bcf1..ee9e7003 100755 --- a/music_assistant/server/controllers/players.py +++ b/music_assistant/server/controllers/players.py @@ -114,6 +114,9 @@ class PlayerController: if player_id in self._players: raise AlreadyRegisteredError(f"Player {player_id} is already registered") + # make sure a default config exists + self.mass.config.create_default_player_config(player_id, player.provider, player.name) + player.enabled = self.mass.config.get(f"{CONF_PLAYERS}/{player_id}/enabled", True) # register playerqueue for this player @@ -516,7 +519,8 @@ class PlayerController: count = 0 while True: count += 1 - for player_id, player in self._players.items(): + for player in list(self._players.values()): + player_id = player.player_id # if the player is playing, update elapsed time every tick # to ensure the queue has accurate details player_playing = ( diff --git a/music_assistant/server/models/provider.py b/music_assistant/server/models/provider.py index c37ac91d..41ee6743 100644 --- a/music_assistant/server/models/provider.py +++ b/music_assistant/server/models/provider.py @@ -28,7 +28,6 @@ class Provider: self.logger = logging.getLogger(f"{ROOT_LOGGER_NAME}.providers.{self.domain}") self.cache = mass.cache self.available = False - self.last_error = None @property def supported_features(self) -> tuple[ProviderFeature, ...]: @@ -90,5 +89,4 @@ class Provider: "instance_id": self.instance_id, "supported_features": [x.value for x in self.supported_features], "available": self.available, - "last_error": self.last_error, } diff --git a/music_assistant/server/providers/airplay/__init__.py b/music_assistant/server/providers/airplay/__init__.py index 3d434403..d565f575 100644 --- a/music_assistant/server/providers/airplay/__init__.py +++ b/music_assistant/server/providers/airplay/__init__.py @@ -16,9 +16,9 @@ import aiofiles from music_assistant.common.models.config_entries import ConfigEntry from music_assistant.common.models.enums import ConfigEntryType -from music_assistant.common.models.errors import PlayerUnavailableError from music_assistant.common.models.player import DeviceInfo, Player from music_assistant.common.models.queue_item import QueueItem +from music_assistant.constants import CONF_PLAYERS from music_assistant.server.models.player_provider import PlayerProvider if TYPE_CHECKING: @@ -284,6 +284,9 @@ class AirplayProvider(PlayerProvider): "-Z", "-d", "all=warn", + # filter out macbooks and apple tv's + "-m", + "macbook,apple-tv,appletv", ] start_success = False while True: @@ -315,6 +318,7 @@ class AirplayProvider(PlayerProvider): async def _check_config_xml(self, recreate: bool = False) -> None: """Check the bridge config XML file.""" + # ruff: noqa: PLR0915 if recreate or not os.path.isfile(self._config_file): if os.path.isfile(self._config_file): os.remove(self._config_file) @@ -349,28 +353,38 @@ class AirplayProvider(PlayerProvider): common_elem.find("sample_rate").text = "44100" common_elem.find("resample").text = "0" common_elem.find("player_volume").text = "20" + + # default values for players + for conf_entry in PLAYER_CONFIG_ENTRIES: + if conf_entry.type == ConfigEntryType.LABEL: + continue + conf_val = conf_entry.default_value + xml_elem = common_elem.find(conf_entry.key) + if xml_elem is None: + xml_elem = ET.SubElement(common_elem, conf_entry.key) + if conf_entry.type == ConfigEntryType.BOOLEAN: + xml_elem.text = "1" if conf_val else "0" + else: + xml_elem.text = str(conf_val) + # get/set all device configs for device_elem in xml_root.findall("device"): player_id = device_elem.find("mac").text - try: - player_conf = self.mass.config.get_player_config(player_id) - except PlayerUnavailableError: - player_conf = None + # use raw config values because players are not + # yet available at startup/init (race condition) + raw_player_conf = self.mass.config.get(f"{CONF_PLAYERS}/{player_id}") + if not raw_player_conf: + continue # prefer name from UDN because default name is often wrong udn = device_elem.find("udn").text udn_name = udn.split("@")[1].split("._")[0] device_elem.find("name").text = udn_name - device_elem.find("enabled").text = ( - "1" if (not player_conf or player_conf.enabled) else "0" - ) + device_elem.find("enabled").text = "1" if raw_player_conf["enabled"] else "0" for conf_entry in PLAYER_CONFIG_ENTRIES: if conf_entry.type == ConfigEntryType.LABEL: continue - if player_conf: - conf_val = player_conf.get_value(conf_entry.key) - else: - conf_val = conf_entry.default_value + conf_val = raw_player_conf["values"].get(conf_entry.key, conf_entry.default_value) xml_elem = device_elem.find(conf_entry.key) if xml_elem is None: xml_elem = ET.SubElement(device_elem, conf_entry.key) diff --git a/music_assistant/server/providers/frontend/__init__.py b/music_assistant/server/providers/frontend/__init__.py index 7a78a46b..3269e112 100644 --- a/music_assistant/server/providers/frontend/__init__.py +++ b/music_assistant/server/providers/frontend/__init__.py @@ -34,4 +34,5 @@ class Frontend(PluginProvider): async def serve_static(self, file_path: str, _request: web.Request) -> web.FileResponse: """Serve file response.""" - return web.FileResponse(file_path) + headers = {"Cache-Control": "no-cache"} + return web.FileResponse(file_path, headers=headers) diff --git a/music_assistant/server/providers/frontend/manifest.json b/music_assistant/server/providers/frontend/manifest.json index e96aba53..a97194fb 100644 --- a/music_assistant/server/providers/frontend/manifest.json +++ b/music_assistant/server/providers/frontend/manifest.json @@ -7,7 +7,7 @@ "config_entries": [ ], - "requirements": ["music-assistant-frontend==20230313.0"], + "requirements": ["music-assistant-frontend==20230317.0"], "documentation": "", "multi_instance": false, "builtin": true, diff --git a/music_assistant/server/providers/slimproto/__init__.py b/music_assistant/server/providers/slimproto/__init__.py index 09ae5698..a969bcd2 100644 --- a/music_assistant/server/providers/slimproto/__init__.py +++ b/music_assistant/server/providers/slimproto/__init__.py @@ -27,11 +27,11 @@ from music_assistant.common.models.enums import ( from music_assistant.common.models.errors import PlayerUnavailableError, QueueEmpty from music_assistant.common.models.player import DeviceInfo, Player from music_assistant.common.models.queue_item import QueueItem +from music_assistant.constants import CONF_PLAYERS from music_assistant.server.models.player_provider import PlayerProvider from music_assistant.server.providers.json_rpc import parse_args if TYPE_CHECKING: - from music_assistant.common.models.config_entries import PlayerConfig from music_assistant.server.providers.json_rpc import JSONRPCApi # sync constants @@ -59,6 +59,8 @@ class SyncPlayPoint: CONF_SYNC_ADJUST = "sync_adjust" +CONF_PLAYER_VOLUME = "player_volume" +DEFAULT_PLAYER_VOLUME = 20 SLIM_PLAYER_CONFIG_ENTRIES = ( ConfigEntry( @@ -71,6 +73,14 @@ SLIM_PLAYER_CONFIG_ENTRIES = ( "and you always hear the audio too late on this player, you can shift the audio a bit.", advanced=True, ), + ConfigEntry( + key=CONF_PLAYER_VOLUME, + type=ConfigEntryType.INTEGER, + default_value=DEFAULT_PLAYER_VOLUME, + label="Default startup volume", + description="Default volume level to set/use when initializing the player.", + advanced=True, + ), ) @@ -80,14 +90,12 @@ class SlimprotoProvider(PlayerProvider): _socket_servers: tuple[asyncio.Server | asyncio.BaseTransport] _socket_clients: dict[str, SlimClient] _sync_playpoints: dict[str, deque[SyncPlayPoint]] - _sync_adjusts: dict[str, int] _virtual_providers: dict[str, tuple[Callable, Callable]] async def setup(self) -> None: """Handle async initialization of the provider.""" self._socket_clients = {} self._sync_playpoints = {} - self._sync_adjusts = {} self._virtual_providers = {} # autodiscovery of the slimproto server does not work # when the port is not the default (3483) so we hardcode it for now @@ -158,14 +166,6 @@ class SlimprotoProvider(PlayerProvider): """Return all (provider/player specific) Config Entries for the given player (if any).""" return SLIM_PLAYER_CONFIG_ENTRIES - def on_player_config_changed( - self, config: PlayerConfig, changed_keys: set[str] # noqa: ARG002 - ) -> None: - """Call (by config manager) when the configuration of a player changes.""" - # during synced playback this value is requested multiple times a second, - # so we cache it in a quick lookup dict - self._sync_adjusts[config.player_id] = config.get_value(CONF_SYNC_ADJUST) - async def cmd_stop(self, player_id: str) -> None: """Send STOP command to given player.""" # forward command to player and any connected sync child's @@ -493,8 +493,11 @@ class SlimprotoProvider(PlayerProvider): # update existing players so they can update their `can_sync_with` field for client in self._socket_clients.values(): self._handle_player_update(client) - # precache player config - self.on_player_config_changed(self.mass.config.get_player_config(player_id), set()) + # handle init/startup volume + init_volume = self.mass.config.get( + f"{CONF_PLAYERS}/{player_id}/{CONF_PLAYER_VOLUME}", DEFAULT_PLAYER_VOLUME + ) + self.mass.create_task(client.volume_set(init_volume)) def _handle_disconnected(self, client: SlimClient) -> None: """Handle a client disconnected event.""" @@ -531,7 +534,9 @@ class SlimprotoProvider(PlayerProvider): def _get_corrected_elapsed_milliseconds(self, client: SlimClient) -> int: """Return corrected elapsed milliseconds.""" - sync_delay = self._sync_adjusts[client.player_id] + sync_delay = self.mass.config.get( + f"{CONF_PLAYERS}/{client.player_id}/{CONF_SYNC_ADJUST}", 0 + ) if sync_delay != 0: return client.elapsed_milliseconds - sync_delay return client.elapsed_milliseconds diff --git a/music_assistant/server/server.py b/music_assistant/server/server.py index dad85ed9..7f527d5c 100644 --- a/music_assistant/server/server.py +++ b/music_assistant/server/server.py @@ -19,7 +19,7 @@ from music_assistant.common.models.enums import EventType, ProviderType from music_assistant.common.models.errors import ProviderUnavailableError, SetupFailedError from music_assistant.common.models.event import MassEvent from music_assistant.common.models.provider import ProviderManifest -from music_assistant.constants import CONF_SERVER_ID, CONF_WEB_IP, ROOT_LOGGER_NAME +from music_assistant.constants import CONF_PROVIDERS, CONF_SERVER_ID, CONF_WEB_IP, ROOT_LOGGER_NAME from music_assistant.server.controllers.cache import CacheController from music_assistant.server.controllers.config import ConfigController from music_assistant.server.controllers.metadata import MetaDataController @@ -298,15 +298,15 @@ class MusicAssistant: """Load (or reload) a provider.""" # if provider is already loaded, stop and unload it first await self.unload_provider(conf.instance_id) - LOGGER.debug("Loading provider %s", conf.name or conf.domain) - # abort if provider is disabled if not conf.enabled: - LOGGER.debug( - "Not loading provider %s because it is disabled", - conf.name or conf.instance_id, - ) - return + raise SetupFailedError("Provider is disabled") + + # validate config + try: + conf.validate() + except (KeyError, ValueError, AttributeError, TypeError) as err: + raise SetupFailedError("Configuration is invalid") from err domain = conf.domain prov_manifest = self._available_providers.get(domain) @@ -316,61 +316,63 @@ class MusicAssistant: raise SetupFailedError( f"Provider {domain} already loaded and only one instance allowed." ) - + # check valid manifest (just in case) if not prov_manifest: raise SetupFailedError(f"Provider {domain} manifest not found") - # try to load the module - try: - prov_mod = importlib.import_module(f".{domain}", "music_assistant.server.providers") - for name, obj in inspect.getmembers(prov_mod): - if not inspect.isclass(obj): - continue - # lookup class to initialize - if name == prov_manifest.init_class or ( - not prov_manifest.init_class - and issubclass( - obj, MusicProvider | PlayerProvider | MetadataProvider | PluginProvider - ) - and obj != MusicProvider - and obj != PlayerProvider - and obj != MetadataProvider - and obj != PluginProvider - ): - prov_cls = obj + # handle dependency on other provider + if prov_manifest.depends_on: + for _ in range(30): + try: + self.get_provider(prov_manifest.depends_on) break + except ProviderUnavailableError: + await asyncio.sleep(1) else: - raise AttributeError("Unable to locate Provider class") - provider: ProviderInstanceType = prov_cls(self, prov_manifest, conf) - self._providers[provider.instance_id] = provider - try: - await provider.setup() - except Exception as err: - provider.last_error = str(err) - provider.available = False - raise err - - # mark provider as available once setup succeeded - provider.available = True - provider.last_error = None - # if this is a music provider, start sync - if provider.type == ProviderType.MUSIC: - await self.music.start_sync(providers=[provider.instance_id]) - # pylint: disable=broad-except - except Exception as exc: - LOGGER.exception( - "Error loading provider(instance) %s: %s", - conf.name or conf.domain, - str(exc), - ) + raise SetupFailedError( + f"Provider {domain} depends on {prov_manifest.depends_on} " + "which is not available." + ) + + # try to load the module + prov_mod = importlib.import_module(f".{domain}", "music_assistant.server.providers") + for name, obj in inspect.getmembers(prov_mod): + if not inspect.isclass(obj): + continue + # lookup class to initialize + if name == prov_manifest.init_class or ( + not prov_manifest.init_class + and issubclass( + obj, MusicProvider | PlayerProvider | MetadataProvider | PluginProvider + ) + and obj != MusicProvider + and obj != PlayerProvider + and obj != MetadataProvider + and obj != PluginProvider + ): + prov_cls = obj + break else: - LOGGER.info( - "Loaded %s provider %s", - provider.type.value, - conf.name or conf.domain, - ) - # always signal event, regardless if the loading succeeded or not + raise AttributeError("Unable to locate Provider class") + provider: ProviderInstanceType = prov_cls(self, prov_manifest, conf) + + try: + await asyncio.wait_for(provider.setup(), 30) + except TimeoutError as err: + raise SetupFailedError(f"Provider {domain} did not load within 30 seconds") from err + # if we reach this point, the provider loaded successfully + LOGGER.info( + "Loaded %s provider %s", + provider.type.value, + conf.name or conf.domain, + ) + provider.available = True + self._providers[provider.instance_id] = provider + self.config.set(f"{CONF_PROVIDERS}/{conf.instance_id}/last_error", None) self.signal_event(EventType.PROVIDERS_UPDATED, data=self.get_providers()) + # if this is a music provider, start sync + if provider.type == ProviderType.MUSIC: + await self.music.start_sync(providers=[provider.instance_id]) async def unload_provider(self, instance_id: str) -> None: """Unload a provider.""" @@ -408,26 +410,33 @@ class MusicAssistant: await self.__load_available_providers() # create default config for any 'load_by_default' providers (e.g. URL provider) - # we must do this first to resolve any dependencies - # NOTE: this will auto load any not yet existing providers - provider_configs = self.config.get_provider_configs() for prov_manifest in self._available_providers.values(): if not prov_manifest.load_by_default: continue - existing = any(x for x in provider_configs if x.domain == prov_manifest.domain) - if existing: - continue - self.config.create_provider_config(prov_manifest.domain, True) + self.config.create_default_provider_config(prov_manifest.domain) + + async def load_provider(prov_conf: ProviderConfig) -> None: + """Try to load a provider and catch errors.""" + try: + await self.load_provider(prov_conf) + # pylint: disable=broad-except + except Exception as exc: + LOGGER.exception( + "Error loading provider(instance) %s: %s", + prov_conf.name or prov_conf.domain, + str(exc), + ) + # if loading failed, we store the error in the config object + # so we can show something useful to the user + prov_conf.last_error = str(exc) + self.config.set(f"{CONF_PROVIDERS}/{prov_conf.instance_id}/last_error", str(exc)) # load all configured (and enabled) providers - for allow_depends_on in (False, True): + async with asyncio.TaskGroup() as tg: for prov_conf in self.config.get_provider_configs(): - prov_manifest = self._available_providers[prov_conf.domain] - if prov_manifest.depends_on and not allow_depends_on: + if not prov_conf.enabled: continue - if prov_conf.instance_id in self._providers: - continue - await self.load_provider(prov_conf) + tg.create_task(load_provider(prov_conf)) async def __load_available_providers(self) -> None: """Preload all available provider manifest files.""" diff --git a/requirements_all.txt b/requirements_all.txt index 7469fc5c..0cf44d48 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -14,7 +14,7 @@ getmac==0.8.2 git+https://github.com/pytube/pytube.git@refs/pull/1501/head mashumaro==3.5.0 memory-tempfile==2.2.3 -music-assistant-frontend==20230313.0 +music-assistant-frontend==20230317.0 orjson==3.8.7 pillow==9.4.0 PyChromecast==13.0.4