Fix: hot reload state synchronization issue in ProviderManager #2796
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
你好 - 我已审阅你的更改 - 以下是一些反馈意见：
- 在验证和自动选择当前提供商（提供商、STT、TTS）方面存在大量几乎相同的逻辑——考虑将其提取到一个共享的辅助函数中以减少重复。
- acm.get_conf("default") 在没有错误处理的情况下被调用——在配置获取失败或没有返回提供商列表的情况下，添加一个备用方案或异常处理。
AI 代理提示
请解决此代码审查中的评论：
## 总体评论
- 在验证和自动选择当前提供商（提供商、STT、TTS）方面存在大量几乎相同的逻辑——考虑将其提取到一个共享的辅助函数中以减少重复。
- acm.get_conf("default") 在没有错误处理的情况下被调用——在配置获取失败或没有返回提供商列表的情况下，添加一个备用方案或异常处理。
## 个人评论
### 评论 1
<location> `astrbot/core/provider/manager.py:403` </location>
<code_context>
await self.terminate_provider(key)
- if len(self.provider_insts) == 0:
+ # 确保当前 Provider 实例仍然有效
+ if (
+ self.curr_provider_inst
</code_context>
<issue_to_address>
**问题 (复杂性):** 考虑将重复的提供商选择逻辑重构为一个单独的辅助方法，以减少重复。
```suggestion
# In your class, add a small helper to DRY up “ensure valid + auto-select” logic:
def _sync_current(self, inst_list, curr_attr_name, provider_name):
curr = getattr(self, curr_attr_name)
# invalidate if missing
if curr and curr.meta().id not in self.inst_map:
curr = None
# auto-select first if none
if curr is None:
if inst_list:
curr = inst_list[0]
logger.info(f"自动选择 {curr.meta().id} 作为当前{provider_name}提供商适配器。")
else:
curr = None
setattr(self, curr_attr_name, curr)
# Then in reload(), replace the three blocks with:
self._sync_current(self.provider_insts, 'curr_provider_inst', '提供商')
self._sync_current(self.stt_provider_insts, 'curr_stt_provider_inst', '语音转文本')
self._sync_current(self.tts_provider_insts, 'curr_tts_provider_inst', '文本转语音')
```
这保留了现有行为，但将重复的“如果丢失则失效 + 选择第一个”模式合并为一个简洁的辅助函数。
</issue_to_address>帮助我更有用！请点击每个评论上的 👍 或 👎，我将使用反馈来改进你的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- There’s a lot of nearly identical logic for validating and auto-selecting current providers (provider, STT, TTS)—consider extracting that into a shared helper to reduce duplication.
- acm.get_conf("default") is called without error handling—add a fallback or exception handling in case the config fetch fails or returns no provider list.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of nearly identical logic for validating and auto-selecting current providers (provider, STT, TTS)—consider extracting that into a shared helper to reduce duplication.
- acm.get_conf("default") is called without error handling—add a fallback or exception handling in case the config fetch fails or returns no provider list.
## Individual Comments
### Comment 1
<location> `astrbot/core/provider/manager.py:403` </location>
<code_context>
await self.terminate_provider(key)
- if len(self.provider_insts) == 0:
+ # 确保当前 Provider 实例仍然有效
+ if (
+ self.curr_provider_inst
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the repeated provider selection logic into a single helper method to reduce duplication.
```suggestion
# In your class, add a small helper to DRY up “ensure valid + auto-select” logic:
def _sync_current(self, inst_list, curr_attr_name, provider_name):
curr = getattr(self, curr_attr_name)
# invalidate if missing
if curr and curr.meta().id not in self.inst_map:
curr = None
# auto-select first if none
if curr is None:
if inst_list:
curr = inst_list[0]
logger.info(f"自动选择 {curr.meta().id} 作为当前{provider_name}提供商适配器。")
else:
curr = None
setattr(self, curr_attr_name, curr)
# Then in reload(), replace the three blocks with:
self._sync_current(self.provider_insts, 'curr_provider_inst', '提供商')
self._sync_current(self.stt_provider_insts, 'curr_stt_provider_inst', '语音转文本')
self._sync_current(self.tts_provider_insts, 'curr_tts_provider_inst', '文本转语音')
```
This preserves existing behavior but collapses the repetitive “invalidate if gone + pick first” pattern into one concise helper.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| def _sync_current_provider(self, inst_list, curr_attr_name, provider_name): | ||
| """同步当前提供商实例，确保其有效性并在必要时自动选择第一个可用实例""" | ||
| curr = getattr(self, curr_attr_name) |
There was a problem hiding this comment.
为什么不直接引用 self.curr_provider_inst 这些
| if provider_config["enable"]: | ||
| await self.load_provider(provider_config) | ||
|
|
||
| # 重新获取最新的配置，增加错误处理 |
There was a problem hiding this comment.
self.providers_config 持有的是 AstrBotConfig 中 providers 的引用，如果 astrbot_config["providers"] 没有被直接覆盖，这里应该不会造成你说的没有更新的问题吧。而 reload 这个方法目前只被 ConfigRoute 中的 post_update_provider 引用，
async def post_update_provider(self):
update_provider_config = await request.json
provider_id = update_provider_config.get("id", None)
new_config = update_provider_config.get("config", None)
if not provider_id or not new_config:
return Response().error("参数错误").__dict__
for i, provider in enumerate(self.config["provider"]):
if provider["id"] == provider_id:
self.config["provider"][i] = new_config
break
else:
return Response().error("未找到对应服务提供商").__dict__
try:
save_config(self.config, self.config, is_core=True)
await self.core_lifecycle.provider_manager.reload(new_config)
except Exception as e:
return Response().error(str(e)).__dict__
return Response().ok(None, "更新成功，已经实时生效~").__dict__是直接对 self.config["provider"] 列表对象修改的
There was a problem hiding this comment.
#2763
我是遇到了这个问题。
我也是一样的情况，我发现这样改就不会出现这个问题了。
至于原因我也没有很深入地研究。
根据我log输出的结果是，增减之后他的列表provider仍然是初始化的列表，而不是修改后的provider。
更改provider信息的时候，provider实例本身是马上更新了，但是provider列表没有更新。导致找不到这个provider的id。
项目太庞大了，我没有很认真的研究，我是局限于我认识空间想办法解决，大佬觉得最佳的更新方法是什么？应该有一行代码立刻更新这个列表的方法，但是我没找到准确位置。
There was a problem hiding this comment.
self.providers_config持有的是 AstrBotConfig 中 providers 的引用，如果 astrbot_config["providers"] 没有被直接覆盖，这里应该不会造成你说的没有更新的问题吧。而 reload 这个方法目前只被 ConfigRoute 中的 post_update_provider 引用，async def post_update_provider(self): update_provider_config = await request.json provider_id = update_provider_config.get("id", None) new_config = update_provider_config.get("config", None) if not provider_id or not new_config: return Response().error("参数错误").__dict__ for i, provider in enumerate(self.config["provider"]): if provider["id"] == provider_id: self.config["provider"][i] = new_config break else: return Response().error("未找到对应服务提供商").__dict__ try: save_config(self.config, self.config, is_core=True) await self.core_lifecycle.provider_manager.reload(new_config) except Exception as e: return Response().error(str(e)).__dict__ return Response().ok(None, "更新成功，已经实时生效~").__dict__是直接对 self.config["provider"] 列表对象修改的
经过我的多重分析。
提供商有3个方法。
新增，删除，更新。
但是新增的时候，如果不合法（比如没有api key），他会添加一个不存在于inst_mapd的提供商。
而更新提供商的时候，却不会重新检查合法性。
结果就是新增了一个永远都不会可能再次被加入inst_mapd的提供商，因为唯一进入提供商的方法只有新增，但是新增却可能新增一个不在inst_mapd的提供商。
我这个补丁是在更新方法里，每次更新的时候强制加入所有提供商，只是一个亡羊补牢。
你原先的设计目的在于不让无效的提供商加入inst_mapd。
但是我觉得这个毫无意义，因为哪怕有API KEY，也可能是无效的。
有没有效果不应该在设置提供商的时候给，因为新增的时候有效，后面也可能有效。
我觉得最简单的方法是新增的时候不应该检查任何的合法性，直接加入inst_mapd。
这个交由您来判断，这个pr作废。
但是这个涉及到了设计逻辑，我认为这个需要大佬您亲自决定应该如何修改。
There was a problem hiding this comment.
#2763 我是遇到了这个问题。 我也是一样的情况，我发现这样改就不会出现这个问题了。 至于原因我也没有很深入地研究。
根据我log输出的结果是，增减之后他的列表provider仍然是初始化的列表，而不是修改后的provider。
更改provider信息的时候，provider实例本身是马上更新了，但是provider列表没有更新。导致找不到这个provider的id。
项目太庞大了，我没有很认真的研究，我是局限于我认识空间想办法解决，大佬觉得最佳的更新方法是什么？应该有一行代码立刻更新这个列表的方法，但是我没找到准确位置。
其实我也一直有遇到过这个问题，包括社区群里面也有不少人反馈这个问题，但是由于不是必现所以我一直没排查出来。
2 participants
Motivation / 动机
reload不会更新服务提供商列表，只会在已有的里面更新
Modifications / 改动点
reload的时候重新加载服务提供商
Verification Steps / 验证步骤
就是更新一下
Screenshots or Test Results / 运行截图或测试结果
没
Compatibility & Breaking Changes / 兼容性与破坏性变更
无变化
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Sourcery 总结
错误修复：
ProviderManager.reload中重新加载完整的提供者列表，以保持内存状态与最新配置同步。Original summary in English
Sourcery 总结
在重新加载时获取并应用最新的提供商配置，并协调内存中的提供商实例以反映这些更改。
错误修复：
Original summary in English
Summary by Sourcery
Fetch and apply the latest provider configuration on reload and reconcile in-memory provider instances to reflect those changes.
Bug Fixes: