Skip to content

Add type hints to config controller functions #2639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Add type hints to config controller functions #2639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 119 additions & 7 deletions music_assistant/controllers/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import base64
import logging
import os
from typing import TYPE_CHECKING, Any, cast
from typing import TYPE_CHECKING, Any, TypeVar, cast, overload
from uuid import uuid4

import aiofiles
Expand Down Expand Up @@ -82,6 +82,9 @@

BASE_KEYS = ("enabled", "name", "available", "default_name", "provider", "type")

# TypeVar for config value type inference
_ConfigValueT = TypeVar("_ConfigValueT", bound=ConfigValueType)

isfile = wrap(os.path.isfile)
remove = wrap(os.remove)
rename = wrap(os.rename)
Expand Down Expand Up @@ -229,9 +232,31 @@ async def get_provider_config(self, instance_id: str) -> ProviderConfig:
msg = f"No config found for provider id {instance_id}"
raise KeyError(msg)

@overload
async def get_provider_config_value(
self, instance_id: str, key: str, *, return_type: type[_ConfigValueT] = ...
) -> _ConfigValueT: ...

@overload
async def get_provider_config_value(
self, instance_id: str, key: str, *, return_type: None = ...
) -> ConfigValueType: ...

@api_command("config/providers/get_value")
async def get_provider_config_value(self, instance_id: str, key: str) -> ConfigValueType:
"""Return single configentry value for a provider."""
async def get_provider_config_value(
self,
instance_id: str,
key: str,
*,
return_type: type[_ConfigValueT | ConfigValueType] | None = None,
) -> _ConfigValueT | ConfigValueType:
"""
Return single configentry value for a provider.

:param instance_id: The provider instance ID.
:param key: The config key to retrieve.
:param return_type: Optional type hint for type inference (e.g., str, int, bool).
"""
Comment on lines +253 to +259
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The return_type parameter is used purely for type inference and doesn't perform runtime type validation. This means if a caller specifies return_type=str but the actual config value is an int, the type checker will assume it's a str while the runtime value will be an int, potentially causing type safety issues.

Consider adding a note in the documentation to clarify that return_type is for static type checking only and callers are responsible for ensuring the specified type matches the actual config value type. Alternatively, consider adding runtime validation with isinstance() to catch type mismatches early.

Copilot uses AI. Check for mistakes.
cache_key = f"prov_conf_value_{instance_id}.{key}"
if (cached_value := self._value_cache.get(cache_key)) is not None:
return cached_value
Expand Down Expand Up @@ -481,14 +506,40 @@ async def get_player_config_entries(

return await player.get_config_entries(action=action, values=values)

@overload
async def get_player_config_value(
self,
player_id: str,
key: str,
unpack_splitted_values: bool = ...,
return_type: type[_ConfigValueT] = ...,
) -> _ConfigValueT: ...

@overload
async def get_player_config_value(
self,
player_id: str,
key: str,
unpack_splitted_values: bool = ...,
return_type: None = ...,
) -> ConfigValueType | tuple[str, ...] | list[tuple[str, ...]]: ...
Comment on lines +509 to +525
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overloads don't correctly handle the interaction between unpack_splitted_values and return_type. When unpack_splitted_values=True, the function always returns tuple[str, ...] | list[tuple[str, ...]] (from line 545) regardless of the return_type parameter. The overloads should be structured as:

  1. When unpack_splitted_values=True → return tuple[str, ...] | list[tuple[str, ...]]
  2. When unpack_splitted_values=False and return_type=type[T] → return T
  3. When unpack_splitted_values=False and return_type=None → return ConfigValueType

This requires using Literal[True] and Literal[False] in the overloads to properly discriminate the return type based on the unpack_splitted_values parameter.

Copilot uses AI. Check for mistakes.

@api_command("config/players/get_value")
async def get_player_config_value(
self,
player_id: str,
key: str,
unpack_splitted_values: bool = False,
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return_type parameter should be keyword-only (using *, before it) to be consistent with the other config getter methods (get_provider_config_value and get_core_config_value), which both use keyword-only parameters for return_type. This ensures API consistency and prevents potential errors from positional argument ordering.

Additionally, usage in this file should be updated to use keyword argument syntax: return_type=str instead of passing str positionally.

Suggested change
unpack_splitted_values: bool = False,
unpack_splitted_values: bool = False,
*,

Copilot uses AI. Check for mistakes.
) -> ConfigValueType | tuple[str, ...] | list[tuple[str, ...]]:
"""Return single configentry value for a player."""
return_type: type[_ConfigValueT | ConfigValueType] | None = None,
) -> _ConfigValueT | ConfigValueType | tuple[str, ...] | list[tuple[str, ...]]:
"""
Return single configentry value for a player.

:param player_id: The player ID.
:param key: The config key to retrieve.
:param unpack_splitted_values: Whether to unpack multi-value config entries.
:param return_type: Optional type hint for type inference (e.g., str, int, bool).
"""
conf = await self.get_player_config(player_id)
if unpack_splitted_values:
return conf.values[key].get_splitted_values()
Expand All @@ -498,6 +549,19 @@ async def get_player_config_value(
else conf.values[key].default_value
)

if TYPE_CHECKING:
# Overload for when default is provided - return type matches default type
@overload
def get_raw_player_config_value(
self, player_id: str, key: str, default: _ConfigValueT
) -> _ConfigValueT: ...

# Overload for when no default is provided - return ConfigValueType | None
@overload
def get_raw_player_config_value(
self, player_id: str, key: str, default: None = None
) -> ConfigValueType | None: ...

def get_raw_player_config_value(
self, player_id: str, key: str, default: ConfigValueType = None
) -> ConfigValueType:
Expand Down Expand Up @@ -806,9 +870,31 @@ async def get_core_config(self, domain: str) -> CoreConfig:
config_entries = await self.get_core_config_entries(domain)
return cast("CoreConfig", CoreConfig.parse(config_entries, raw_conf))

@overload
async def get_core_config_value(
self, domain: str, key: str, *, return_type: type[_ConfigValueT] = ...
) -> _ConfigValueT: ...

@overload
async def get_core_config_value(
self, domain: str, key: str, *, return_type: None = ...
) -> ConfigValueType: ...

@api_command("config/core/get_value")
async def get_core_config_value(self, domain: str, key: str) -> ConfigValueType:
"""Return single configentry value for a core controller."""
async def get_core_config_value(
self,
domain: str,
key: str,
*,
return_type: type[_ConfigValueT | ConfigValueType] | None = None,
) -> _ConfigValueT | ConfigValueType:
"""
Return single configentry value for a core controller.

:param domain: The core controller domain.
:param key: The config key to retrieve.
:param return_type: Optional type hint for type inference (e.g., str, int, bool).
"""
conf = await self.get_core_config(domain)
return (
conf.values[key].value
Expand Down Expand Up @@ -862,6 +948,19 @@ async def save_core_config(
# return full config, just in case
return await self.get_core_config(domain)

if TYPE_CHECKING:
# Overload for when default is provided - return type matches default type
@overload
def get_raw_core_config_value(
self, core_module: str, key: str, default: _ConfigValueT
) -> _ConfigValueT: ...

# Overload for when no default is provided - return ConfigValueType | None
@overload
def get_raw_core_config_value(
self, core_module: str, key: str, default: None = None
) -> ConfigValueType | None: ...

def get_raw_core_config_value(
self, core_module: str, key: str, default: ConfigValueType = None
) -> ConfigValueType:
Expand All @@ -878,6 +977,19 @@ def get_raw_core_config_value(
),
)

if TYPE_CHECKING:
# Overload for when default is provided - return type matches default type
@overload
def get_raw_provider_config_value(
self, provider_instance: str, key: str, default: _ConfigValueT
) -> _ConfigValueT: ...

# Overload for when no default is provided - return ConfigValueType | None
@overload
def get_raw_provider_config_value(
self, provider_instance: str, key: str, default: None = None
) -> ConfigValueType | None: ...

def get_raw_provider_config_value(
self, provider_instance: str, key: str, default: ConfigValueType = None
) -> ConfigValueType:
Expand Down
5 changes: 2 additions & 3 deletions music_assistant/controllers/music.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1533,9 +1533,8 @@ async def _schedule_provider_mediatype_sync(
if not sync_conf:
return
conf_key = f"provider_sync_interval_{media_type.value}s"
sync_interval = cast(
"int",
await self.mass.config.get_provider_config_value(provider.instance_id, conf_key),
sync_interval = await self.mass.config.get_provider_config_value(
provider.instance_id, conf_key, return_type=int
)
if sync_interval <= 0:
# sync disabled for this media type
Expand Down
4 changes: 2 additions & 2 deletions music_assistant/providers/airplay/protocols/raop.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ async def start(self, start_ntp: int) -> None:
if prop_value := self.player.raop_discovery_info.decoded_properties.get(prop):
extra_args += [f"-{prop}", prop_value]
if device_password := self.mass.config.get_raw_player_config_value(
player_id, CONF_PASSWORD, None
player_id, CONF_PASSWORD
):
extra_args += ["-password", str(device_password)]
# Add AirPlay credentials from pairing if available (for Apple devices)
Expand All @@ -64,7 +64,7 @@ async def start(self, start_ntp: int) -> None:
elif self.prov.logger.isEnabledFor(VERBOSE_LOG_LEVEL):
extra_args += ["-debug", "10"]
read_ahead = await self.mass.config.get_player_config_value(
player_id, CONF_READ_AHEAD_BUFFER
player_id, CONF_READ_AHEAD_BUFFER, return_type=int
)

# cliraop is the binary that handles the actual raop streaming to the player
Expand Down
7 changes: 3 additions & 4 deletions music_assistant/providers/squeezelite/provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,10 @@ async def unload(self, is_removed: bool = False) -> None:

def get_corrected_elapsed_milliseconds(self, slimplayer: SlimClient) -> int:
"""Return corrected elapsed milliseconds for a slimplayer."""
sync_delay = cast(
"int",
self.mass.config.get_raw_player_config_value(slimplayer.player_id, CONF_SYNC_ADJUST, 0),
sync_delay = self.mass.config.get_raw_player_config_value(
slimplayer.player_id, CONF_SYNC_ADJUST, 0
)
return cast("int", slimplayer.elapsed_milliseconds - sync_delay)
return int(slimplayer.elapsed_milliseconds - sync_delay)

def _handle_slimproto_event(
self,
Expand Down
5 changes: 2 additions & 3 deletions music_assistant/providers/universal_group/player.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,8 @@ async def _serve_ugp_stream(self, request: web.Request) -> web.StreamResponse:
content_sample_rate=UGP_FORMAT.sample_rate,
content_bit_depth=UGP_FORMAT.bit_depth,
)
http_profile = cast(
"str",
await self.mass.config.get_player_config_value(child_player_id, CONF_HTTP_PROFILE),
http_profile = await self.mass.config.get_player_config_value(
child_player_id, CONF_HTTP_PROFILE, False, str
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return_type parameter should be passed as a keyword argument (return_type=str) instead of a positional argument for consistency with similar usages elsewhere in the codebase (e.g., airplay/protocols/raop.py line 67) and to match the API design of other config getter methods.

Suggested change
child_player_id, CONF_HTTP_PROFILE, False, str
child_player_id, CONF_HTTP_PROFILE, False, return_type=str

Copilot uses AI. Check for mistakes.
)
elif output_format_str == "flac":
output_format = AudioFormat(content_type=ContentType.FLAC)
Expand Down
Loading