Don't force reload on all config changes (music-assistant#3045)

Author: apophisnow

music_assistant/constants.py

@@ -159,6 +159,7 @@
     ],
     default_value="GLOBAL",
     category="advanced",
+    requires_reload=False,  # applied dynamically via _set_logger()
 )
 
 DEFAULT_PROVIDER_CONFIG_ENTRIES = (CONF_ENTRY_LOG_LEVEL,)
@@ -172,6 +173,7 @@
     label="Enforce Gapless playback with Queue Flow Mode streaming",
     default_value=False,
     category="advanced",
+    requires_reload=True,
 )
 
 
@@ -583,6 +585,7 @@ def create_output_codec_config_entry(
     ],
     default_value="default",
     category="advanced",
+    requires_reload=True,
 )
 CONF_ENTRY_LIBRARY_SYNC_ALBUMS = ConfigEntry(
     key="library_sync_albums",

music_assistant/controllers/streams/streams_controller.py

@@ -262,6 +262,7 @@ async def get_config_entries(
                 "otherwise audio streaming will not work.",
                 required=False,
                 category="advanced",
+                requires_reload=True,
             ),
             ConfigEntry(
                 key=CONF_BIND_PORT,
@@ -272,6 +273,7 @@ async def get_config_entries(
                 "Make sure that this server can be reached "
                 "on the given IP and TCP port by players on the local network.",
                 category="advanced",
+                requires_reload=True,
             ),
             ConfigEntry(
                 key=CONF_BIND_IP,
@@ -285,6 +287,7 @@ async def get_config_entries(
                 "not be adjusted in regular setups.",
                 category="advanced",
                 required=False,
+                requires_reload=True,
             ),
             ConfigEntry(
                 key=CONF_SMART_FADES_LOG_LEVEL,

music_assistant/controllers/webserver/auth.py

@@ -25,7 +25,6 @@
 )
 
 from music_assistant.constants import (
-    CONF_AUTH_ALLOW_SELF_REGISTRATION,
     DB_TABLE_PLAYLOG,
     HOMEASSISTANT_SYSTEM_USER,
     MASS_LOGGER_NAME,
@@ -40,7 +39,6 @@
     HomeAssistantOAuthProvider,
     HomeAssistantProviderConfig,
     LoginProvider,
-    LoginProviderConfig,
     normalize_username,
 )
 from music_assistant.helpers.api import api_command
@@ -81,10 +79,6 @@ def __init__(self, webserver: WebserverController) -> None:
 
     async def setup(self) -> None:
         """Initialize the authentication manager."""
-        # Get auth settings from config
-        allow_self_registration = self.webserver.config.get_value(CONF_AUTH_ALLOW_SELF_REGISTRATION)
-        assert isinstance(allow_self_registration, bool)
-
         # Setup database
         db_path = self.mass.storage_path + "/auth.db"
         self.database = DatabaseConnection(db_path)
@@ -97,8 +91,8 @@ async def setup(self) -> None:
         jwt_secret = await self._get_or_create_jwt_secret()
         self.jwt_helper = JWTHelper(jwt_secret)
 
-        # Setup login providers based on config
-        await self._setup_login_providers(allow_self_registration)
+        # Setup login providers
+        await self._setup_login_providers()
 
         self._has_users = await self._has_non_system_users()
 
@@ -286,15 +280,10 @@ async def _get_or_create_jwt_secret(self) -> str:
         self.logger.info("Generated new JWT secret key")
         return jwt_secret
 
-    async def _setup_login_providers(self, allow_self_registration: bool) -> None:
-        """
-        Set up available login providers based on configuration.
-
-        :param allow_self_registration: Whether to allow self-registration via OAuth.
-        """
+    async def _setup_login_providers(self) -> None:
+        """Set up available login providers based on configuration."""
         # Always enable built-in provider
-        builtin_config: LoginProviderConfig = {"allow_self_registration": False}
-        self.login_providers["builtin"] = BuiltinLoginProvider(self.mass, "builtin", builtin_config)
+        self.login_providers["builtin"] = BuiltinLoginProvider(self.mass, "builtin", {})
 
         # Home Assistant OAuth provider
         # Automatically enabled if HA provider (plugin) is configured
@@ -308,10 +297,7 @@ async def _setup_login_providers(self, allow_self_registration: bool) -> None:
             # Get URL from the HA provider config
             ha_url = ha_provider.config.get_value("url")
             assert isinstance(ha_url, str)
-            ha_config: HomeAssistantProviderConfig = {
-                "ha_url": ha_url,
-                "allow_self_registration": allow_self_registration,
-            }
+            ha_config: HomeAssistantProviderConfig = {"ha_url": ha_url}
             self.login_providers["homeassistant"] = HomeAssistantOAuthProvider(
                 self.mass, "homeassistant", ha_config
             )
@@ -336,18 +322,10 @@ async def _sync_ha_oauth_provider(self) -> None:
         if ha_provider:
             # HA provider exists and is available - ensure OAuth provider is registered
             if "homeassistant" not in self.login_providers:
-                # Get allow_self_registration config
-                allow_self_registration = bool(
-                    self.webserver.config.get_value(CONF_AUTH_ALLOW_SELF_REGISTRATION, True)
-                )
-
                 # Get URL from the HA provider config
                 ha_url = ha_provider.config.get_value("url")
                 assert isinstance(ha_url, str)
-                ha_config: HomeAssistantProviderConfig = {
-                    "ha_url": ha_url,
-                    "allow_self_registration": allow_self_registration,
-                }
+                ha_config: HomeAssistantProviderConfig = {"ha_url": ha_url}
                 self.login_providers["homeassistant"] = HomeAssistantOAuthProvider(
                     self.mass, "homeassistant", ha_config
                 )

music_assistant/controllers/webserver/controller.py

@@ -98,8 +98,8 @@ def __init__(self, mass: MusicAssistant) -> None:
 
     @property
     def base_url(self) -> str:
-        """Return the base_url for the streamserver."""
-        return self._server.base_url
+        """Return the base_url for the webserver."""
+        return str(self.config.get_value(CONF_BASE_URL)).removesuffix("/")
 
     async def get_config_entries(
         self,
@@ -131,6 +131,7 @@ async def get_config_entries(
                 label="Allow User Self-Registration",
                 description="Allow users to create accounts via Home Assistant OAuth.",
                 hidden=not any(provider.domain == "hass" for provider in self.mass.providers),
+                requires_reload=False,
             ),
             ConfigEntry(
                 key=CONF_BASE_URL,
@@ -140,13 +141,15 @@ async def get_config_entries(
                 description="The (base) URL to reach this webserver in the network. \n"
                 "Override this in advanced scenarios where for example you're running "
                 "the webserver behind a reverse proxy.",
+                requires_reload=False,
             ),
             ConfigEntry(
                 key=CONF_BIND_PORT,
                 type=ConfigEntryType.INTEGER,
                 default_value=DEFAULT_SERVER_PORT,
                 label="TCP Port",
                 description="The TCP port to run the webserver.",
+                requires_reload=True,
             ),
             ConfigEntry(
                 key="webserver_warn",
@@ -169,6 +172,7 @@ async def get_config_entries(
                 label="Enable SSL/TLS",
                 description="Enable HTTPS by providing an SSL certificate and private key. \n"
                 "This encrypts all communication with the webserver.",
+                requires_reload=True,
             ),
             ConfigEntry(
                 key=CONF_SSL_CERTIFICATE,
@@ -181,6 +185,7 @@ async def get_config_entries(
                 "Both RSA and ECDSA certificates are supported.",
                 required=False,
                 depends_on=CONF_ENABLE_SSL,
+                requires_reload=True,
             ),
             ConfigEntry(
                 key=CONF_SSL_PRIVATE_KEY,
@@ -193,6 +198,7 @@ async def get_config_entries(
                 "This is securely encrypted and stored.",
                 required=False,
                 depends_on=CONF_ENABLE_SSL,
+                requires_reload=True,
             ),
             ConfigEntry(
                 key=CONF_ACTION_VERIFY_SSL,
@@ -227,6 +233,7 @@ async def get_config_entries(
                 "This is an advanced setting that should normally "
                 "not be adjusted in regular setups.",
                 category="advanced",
+                requires_reload=True,
             ),
         )
 

music_assistant/controllers/webserver/helpers/auth_providers.py

@@ -18,7 +18,7 @@
 from music_assistant_models.auth import AuthProviderType, User, UserRole
 from music_assistant_models.errors import AuthenticationFailed
 
-from music_assistant.constants import MASS_LOGGER_NAME
+from music_assistant.constants import CONF_AUTH_ALLOW_SELF_REGISTRATION, MASS_LOGGER_NAME
 from music_assistant.helpers.datetime import utc
 
 if TYPE_CHECKING:
@@ -252,8 +252,6 @@ async def clear_attempts(self, username: str) -> None:
 class LoginProviderConfig(TypedDict, total=False):
     """Base configuration for login providers."""
 
-    allow_self_registration: bool
-
 
 class HomeAssistantProviderConfig(LoginProviderConfig):
     """Configuration for Home Assistant OAuth provider."""
@@ -287,7 +285,11 @@ def __init__(self, mass: MusicAssistant, provider_id: str, config: LoginProvider
         self.provider_id = provider_id
         self.config = config
         self.logger = LOGGER
-        self.allow_self_registration = config.get("allow_self_registration", False)
+
+    @property
+    def allow_self_registration(self) -> bool:
+        """Return whether self-registration is allowed for this provider."""
+        return False
 
     @property
     def auth_manager(self) -> AuthenticationManager:
@@ -520,6 +522,11 @@ def __init__(self, mass: MusicAssistant, provider_id: str, config: LoginProvider
         # Store OAuth state -> return_url mapping to support concurrent sessions
         self._oauth_sessions: dict[str, str | None] = {}
 
+    @property
+    def allow_self_registration(self) -> bool:
+        """Return whether self-registration is allowed, read dynamically from config."""
+        return bool(self.mass.webserver.config.get_value(CONF_AUTH_ALLOW_SELF_REGISTRATION))
+
     @property
     def provider_type(self) -> AuthProviderType:
         """Return the provider type."""

music_assistant/models/core_controller.py

@@ -63,13 +63,22 @@ async def reload(self, config: CoreConfig | None = None) -> None:
 
     async def update_config(self, config: CoreConfig, changed_keys: set[str]) -> None:
         """Handle logic when the config is updated."""
-        # default implementation: perform a full reload on any config change
-        # TODO: only reload when 'requires_reload' keys changed
-        if changed_keys == {f"values/{CONF_LOG_LEVEL}"}:
-            # only log level changed, no need to reload
+        # always update the stored config so dynamic reads pick up new values
+        self.config = config
+
+        # apply log level change dynamically (doesn't require reload)
+        if f"values/{CONF_LOG_LEVEL}" in changed_keys:
             log_value = str(config.get_value(CONF_LOG_LEVEL))
             self._set_logger(log_value)
-        else:
+
+        # reload if any changed value entry has requires_reload set to True
+        needs_reload = any(
+            (entry := config.values.get(key.removeprefix("values/"))) is not None
+            and entry.requires_reload is True
+            for key in changed_keys
+            if key.startswith("values/")
+        )
+        if needs_reload:
             self.logger.info(
                 "Config updated, reloading %s core controller",
                 self.manifest.name,

music_assistant/models/provider.py

@@ -65,14 +65,23 @@ async def update_config(self, config: ProviderConfig, changed_keys: set[str]) ->
         Override this method in your provider implementation if you need
         to perform any additional setup logic after the provider is registered and
         the self.config was loaded, and whenever the config changes.
+
+        The default implementation reloads the provider on any config change
+        (except log-level-only changes), since provider reloads are lightweight
+        and most providers cache config values at setup time.
         """
-        # default implementation: perform a full reload on any config change
-        # override in your provider if you need more fine-grained control
-        # such as checking the changed_keys set and only reload when 'requires_reload' keys changed
-        if changed_keys == {f"values/{CONF_LOG_LEVEL}"}:
-            # only log level changed, no need to reload
+        # always update the stored config so dynamic reads pick up new values
+        self.config = config
+
+        # update log level if changed
+        if f"values/{CONF_LOG_LEVEL}" in changed_keys:
             self._set_log_level_from_config(config)
-        else:
+
+        # reload if any non-log-level value keys changed
+        value_keys_changed = {
+            k for k in changed_keys if k.startswith("values/") and k != f"values/{CONF_LOG_LEVEL}"
+        }
+        if value_keys_changed:
             self.logger.info(
                 "Config updated, reloading provider %s (instance_id=%s)",
                 self.domain,