-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: sync fallback chat models when a provider is deleted or renamed #9171
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -813,6 +813,24 @@ async def terminate_provider(self, provider_id: str) -> None: | |
| ) | ||
| del self.inst_map[provider_id] | ||
|
|
||
| @staticmethod | ||
| def _sync_fallback_chat_models( | ||
| config: dict, old_id: str, new_id: str | None = None | ||
| ) -> None: | ||
| """Remove or remap a provider id in fallback_chat_models after delete/rename.""" | ||
| provider_settings = config.get("provider_settings") | ||
| if not isinstance(provider_settings, dict): | ||
| return | ||
| fallback_ids = provider_settings.get("fallback_chat_models") | ||
| if not isinstance(fallback_ids, list) or old_id not in fallback_ids: | ||
| return | ||
| updated_ids = [] | ||
| for fallback_id in fallback_ids: | ||
| mapped_id = new_id if fallback_id == old_id else fallback_id | ||
| if mapped_id is not None and mapped_id not in updated_ids: | ||
| updated_ids.append(mapped_id) | ||
| provider_settings["fallback_chat_models"] = updated_ids | ||
|
|
||
| async def delete_provider( | ||
| self, provider_id: str | None = None, provider_source_id: str | None = None | ||
| ) -> None: | ||
|
|
@@ -832,6 +850,7 @@ async def delete_provider( | |
| config["provider"] = [ | ||
| prov for prov in config["provider"] if prov.get("id") != tpid | ||
| ] | ||
| self._sync_fallback_chat_models(config, tpid) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| config.save_config() | ||
| logger.info(f"Provider {target_prov_ids} 已从配置中删除。") | ||
|
|
||
|
|
@@ -855,6 +874,8 @@ async def update_provider(self, origin_provider_id: str, new_config: dict) -> No | |
| break | ||
| else: | ||
| raise ValueError(f"Provider ID {origin_provider_id} not found") | ||
| if npid != origin_provider_id: | ||
| self._sync_fallback_chat_models(config, origin_provider_id, npid) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| config.save_config() | ||
| # reload instance | ||
| await self.reload(new_config) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| import asyncio | ||
| from types import SimpleNamespace | ||
|
|
||
| import pytest | ||
|
|
||
| import astrbot.core.star.context # noqa: F401 # 先导入以规避 provider.manager 的循环导入 | ||
| from astrbot.core.provider.manager import ProviderManager | ||
|
|
||
|
|
||
| class FakeConfig(dict): | ||
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
| self.save_count = 0 | ||
|
|
||
| def save_config(self): | ||
| self.save_count += 1 | ||
|
|
||
|
|
||
| def _build_manager(config: FakeConfig) -> ProviderManager: | ||
| manager = ProviderManager.__new__(ProviderManager) | ||
| manager.resource_lock = asyncio.Lock() | ||
| manager.acm = SimpleNamespace(default_conf=config) | ||
| manager.providers_config = config["provider"] | ||
| return manager | ||
|
|
||
|
|
||
| async def _noop_terminate(provider_id: str) -> None: | ||
| del provider_id | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_delete_provider_removes_fallback_reference(): | ||
| config = FakeConfig( | ||
| { | ||
| "provider": [{"id": "p1"}, {"id": "p2"}], | ||
| "provider_settings": {"fallback_chat_models": ["p1", "p2"]}, | ||
| } | ||
| ) | ||
| manager = _build_manager(config) | ||
| manager.terminate_provider = _noop_terminate | ||
|
|
||
| await manager.delete_provider(provider_id="p1") | ||
|
|
||
| assert config["provider"] == [{"id": "p2"}] | ||
| assert config["provider_settings"]["fallback_chat_models"] == ["p2"] | ||
| assert config.save_count == 1 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_delete_provider_source_removes_all_fallback_references(): | ||
| config = FakeConfig( | ||
| { | ||
| "provider": [ | ||
| {"id": "p1", "provider_source_id": "src"}, | ||
| {"id": "p2", "provider_source_id": "src"}, | ||
| {"id": "p3"}, | ||
| ], | ||
| "provider_settings": {"fallback_chat_models": ["p1", "p2", "p3"]}, | ||
| } | ||
| ) | ||
| manager = _build_manager(config) | ||
| manager.terminate_provider = _noop_terminate | ||
|
|
||
| await manager.delete_provider(provider_source_id="src") | ||
|
|
||
| assert config["provider"] == [{"id": "p3"}] | ||
| assert config["provider_settings"]["fallback_chat_models"] == ["p3"] | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_update_provider_renames_fallback_reference(): | ||
| config = FakeConfig( | ||
| { | ||
| "provider": [{"id": "p1", "enable": True}], | ||
| "provider_settings": {"fallback_chat_models": ["p1", "p3"]}, | ||
| } | ||
| ) | ||
| manager = _build_manager(config) | ||
| reloaded = [] | ||
|
|
||
| async def _record_reload(new_config: dict) -> None: | ||
| reloaded.append(new_config) | ||
|
|
||
| manager.reload = _record_reload | ||
|
|
||
| await manager.update_provider("p1", {"id": "p1-renamed", "enable": True}) | ||
|
|
||
| assert config["provider_settings"]["fallback_chat_models"] == ["p1-renamed", "p3"] | ||
| assert reloaded and reloaded[0]["id"] == "p1-renamed" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_update_provider_rename_dedupes_stale_target(): | ||
| # "p2" 是残留在回退列表里的失效 ID;把 p1 改名为 p2 后不应产生重复项 | ||
| config = FakeConfig( | ||
| { | ||
| "provider": [{"id": "p1", "enable": True}], | ||
| "provider_settings": {"fallback_chat_models": ["p1", "p2"]}, | ||
| } | ||
| ) | ||
| manager = _build_manager(config) | ||
|
|
||
| async def _record_reload(new_config: dict) -> None: | ||
| del new_config | ||
|
|
||
| manager.reload = _record_reload | ||
|
|
||
| await manager.update_provider("p1", {"id": "p2", "enable": True}) | ||
|
|
||
| assert config["provider_settings"]["fallback_chat_models"] == ["p2"] | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_update_provider_same_id_keeps_fallback_list(): | ||
| config = FakeConfig( | ||
| { | ||
| "provider": [{"id": "p1", "enable": True}], | ||
| "provider_settings": {"fallback_chat_models": ["p1"]}, | ||
| } | ||
| ) | ||
| manager = _build_manager(config) | ||
|
|
||
| async def _record_reload(new_config: dict) -> None: | ||
| del new_config | ||
|
|
||
| manager.reload = _record_reload | ||
|
|
||
| await manager.update_provider("p1", {"id": "p1", "enable": False}) | ||
|
|
||
| assert config["provider_settings"]["fallback_chat_models"] == ["p1"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 建议增加单元测试,以覆盖 assert config["provider_settings"]["fallback_chat_models"] == ["p1"]
@pytest.mark.asyncio
async def test_update_provider_renames_other_references():
config = FakeConfig(
{
"provider": [{"id": "p1", "enable": True}],
"provider_settings": {
"fallback_chat_models": ["p1"],
"default_provider_id": "p1",
},
"provider_stt_settings": {"provider_id": "p1"},
"provider_tts_settings": {"provider_id": "p1"},
}
)
manager = _build_manager(config)
manager.reload = lambda x: asyncio.sleep(0)
await manager.update_provider("p1", {"id": "p1-renamed", "enable": True})
assert config["provider_settings"]["default_provider_id"] == "p1-renamed"
assert config["provider_stt_settings"]["provider_id"] == "p1-renamed"
assert config["provider_tts_settings"]["provider_id"] == "p1-renamed" |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
当删除或重命名 Provider 时,除了
fallback_chat_models之外,provider_settings.default_provider_id、provider_stt_settings.provider_id以及provider_tts_settings.provider_id也可能会残留失效的旧 ID。这会导致在重命名或删除默认 Provider 后,系统依然尝试加载旧 ID 并打印警告日志。建议将此方法重命名为
_sync_provider_id_references,并同时同步更新这些配置项。