Fix provider duplication by making config properties dynamic #3243
Conversation
Co-authored-by: LIghtJUNction <106986785+LIghtJUNction@users.noreply.github.com>
Co-authored-by: LIghtJUNction <106986785+LIghtJUNction@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR converts static configuration attributes in ProviderManager to dynamic properties to fix a bug where provider configuration changes were not reflected in the manager. Previously, configuration values were assigned once during initialization and never updated, causing the WebUI to fail to see newly added providers.
- Removed static attribute initialization of
providers_config,provider_settings,provider_stt_settings, andprovider_tts_settingsfrom__init__ - Converted these to
@propertymethods that dynamically fetch configuration fromself.acm.confs["default"] - Added comprehensive test coverage for the dynamic property behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| astrbot/core/provider/manager.py | Removed static attribute assignments in __init__ and added four @property methods to make configuration access dynamic |
| tests/test_provider_manager.py | Added new test file with 5 test functions verifying that configuration properties dynamically reflect changes |
Comments suppressed due to low confidence (2)
astrbot/core/provider/manager.py:403
- This line checks
self.provider_settings.get(\"provider_id\")but should checkself.provider_tts_settings.get(\"provider_id\")to match TTS provider settings. The current code incorrectly references the general provider settings instead of the TTS-specific settings.
if self.provider_settings.get("provider_id") == provider_config["id"]:
astrbot/core/provider/manager.py:387
- These property accesses on lines 385, 403, and 424 are now calling getter methods repeatedly. Consider caching the property values at the start of the
load_providermethod to avoid multiple dictionary lookups through the property getters, improving both readability and performance.
self.provider_stt_settings.get("provider_id")
== provider_config["id"]
):
astrbot/core/provider/manager.py
Outdated
| @property | ||
| def providers_config(self) -> list: | ||
| """动态获取最新的 provider 配置列表""" | ||
| config = self.acm.confs["default"] | ||
| return config["provider"] | ||
|
|
||
| @property | ||
| def provider_settings(self) -> dict: | ||
| """动态获取最新的 provider_settings 配置""" | ||
| config = self.acm.confs["default"] | ||
| return config["provider_settings"] | ||
|
|
||
| @property | ||
| def provider_stt_settings(self) -> dict: | ||
| """动态获取最新的 provider_stt_settings 配置""" | ||
| config = self.acm.confs["default"] | ||
| return config.get("provider_stt_settings", {}) | ||
|
|
||
| @property | ||
| def provider_tts_settings(self) -> dict: | ||
| """动态获取最新的 provider_tts_settings 配置""" | ||
| config = self.acm.confs["default"] | ||
| return config.get("provider_tts_settings", {}) |
There was a problem hiding this comment.
The property methods repeatedly access self.acm.confs[\"default\"] on every call. Consider caching the config reference at the class level or using a helper method to avoid redundant dictionary lookups, especially since these properties may be accessed frequently in loops (e.g., line 205 iterates over providers_config).
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Applied the suggested refactoring. Added _default_config property to centralize config access and eliminate code duplication across all four config properties. This addresses both the performance concern and maintainability. (commit 06c4a11)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
astrbot/core/provider/manager.py
Outdated
| @property | ||
| def providers_config(self) -> list: | ||
| """动态获取最新的 provider 配置列表""" | ||
| config = self.acm.confs["default"] | ||
| return config["provider"] |
There was a problem hiding this comment.
The providers_config property accesses self.acm.confs['default'] on every call. This could cause performance issues if this property is accessed frequently in loops (e.g., line 205). Consider caching the config access or using a method with an explicit refresh parameter if the dynamic behavior is only needed in specific scenarios.
| @property | |
| def providers_config(self) -> list: | |
| """动态获取最新的 provider 配置列表""" | |
| config = self.acm.confs["default"] | |
| return config["provider"] | |
| def get_providers_config(self, refresh: bool = True) -> list: | |
| """ | |
| 获取 provider 配置列表。 | |
| :param refresh: 是否动态获取最新配置。为 False 时使用缓存（适合高频场景）。 | |
| """ | |
| if not hasattr(self, "_cached_providers_config") or refresh: | |
| config = self.acm.confs["default"] | |
| self._cached_providers_config = config["provider"] | |
| return self._cached_providers_config |
astrbot/core/provider/manager.py
Outdated
| def providers_config(self) -> list: | ||
| """动态获取最新的 provider 配置列表""" | ||
| config = self.acm.confs["default"] | ||
| return config["provider"] | ||
|
|
||
| @property | ||
| def provider_settings(self) -> dict: | ||
| """动态获取最新的 provider_settings 配置""" | ||
| config = self.acm.confs["default"] | ||
| return config.get("provider_settings", {}) | ||
|
|
||
| @property | ||
| def provider_stt_settings(self) -> dict: | ||
| """动态获取最新的 provider_stt_settings 配置""" | ||
| config = self.acm.confs["default"] | ||
| return config.get("provider_stt_settings", {}) | ||
|
|
||
| @property | ||
| def provider_tts_settings(self) -> dict: | ||
| """动态获取最新的 provider_tts_settings 配置""" | ||
| config = self.acm.confs["default"] | ||
| return config.get("provider_tts_settings", {}) |
There was a problem hiding this comment.
The pattern config = self.acm.confs['default'] is repeated in all four properties (lines 76, 82, 88, 94). Consider extracting this to a private helper method or property (e.g., _get_config()) to improve maintainability and reduce duplication.
| def providers_config(self) -> list: | |
| """动态获取最新的 provider 配置列表""" | |
| config = self.acm.confs["default"] | |
| return config["provider"] | |
| @property | |
| def provider_settings(self) -> dict: | |
| """动态获取最新的 provider_settings 配置""" | |
| config = self.acm.confs["default"] | |
| return config.get("provider_settings", {}) | |
| @property | |
| def provider_stt_settings(self) -> dict: | |
| """动态获取最新的 provider_stt_settings 配置""" | |
| config = self.acm.confs["default"] | |
| return config.get("provider_stt_settings", {}) | |
| @property | |
| def provider_tts_settings(self) -> dict: | |
| """动态获取最新的 provider_tts_settings 配置""" | |
| config = self.acm.confs["default"] | |
| return config.get("provider_tts_settings", {}) | |
| def _default_config(self) -> dict: | |
| """私有属性，获取默认配置""" | |
| return self.acm.confs["default"] | |
| @property | |
| def providers_config(self) -> list: | |
| """动态获取最新的 provider 配置列表""" | |
| return self._default_config["provider"] | |
| @property | |
| def provider_settings(self) -> dict: | |
| """动态获取最新的 provider_settings 配置""" | |
| return self._default_config.get("provider_settings", {}) | |
| @property | |
| def provider_stt_settings(self) -> dict: | |
| """动态获取最新的 provider_stt_settings 配置""" | |
| return self._default_config.get("provider_stt_settings", {}) | |
| @property | |
| def provider_tts_settings(self) -> dict: | |
| """动态获取最新的 provider_tts_settings 配置""" | |
| return self._default_config.get("provider_tts_settings", {}) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: LIghtJUNction <106986785+LIghtJUNction@users.noreply.github.com>
providers_configin ProviderManager is cached and not updated when new providers are addedSummary
Issue #3205: Users could not add duplicate/copy of the same service provider. When attempting to add a provider copy (e.g.,
deepseek-st_copy), it would be added initially but disappear after any provider update operation.Root Cause:
The
ProviderManager.providers_configand related config properties were cached during initialization and never updated, causing stale data issues.Solution:
Converted config properties to dynamic
@propertymethods that read from the config manager. Added a private_default_configproperty to centralize and optimize the repeated config access pattern.Latest Changes:
_default_configproperty to reduce code duplication and improve maintainabilityself.acm.confs["default"]Validation:
✅ All tests pass (5/5)
✅ Linting passes (ruff)
✅ Code follows suggested improvements from review
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.