refactor(core): restructure initialization logic in __init__.py for…#8954
Conversation
… global instances
|
@Initial01LingXu is attempting to deploy a commit to the soulter's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
_bootstrapfunction is not thread-safe: concurrent first-time accesses to lazy attributes could race and initialize globals multiple times; consider adding a simple lock/guard (e.g.,threading.Lock) around the initialization block. - Instead of constructing
__dir__fromglobals()each call and appending_lazy_attrs, consider defining an explicit__all__(including the lazy attributes) and basing__dir__on that list to make the public API more predictable and avoid potential duplicates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_bootstrap` function is not thread-safe: concurrent first-time accesses to lazy attributes could race and initialize globals multiple times; consider adding a simple lock/guard (e.g., `threading.Lock`) around the initialization block.
- Instead of constructing `__dir__` from `globals()` each call and appending `_lazy_attrs`, consider defining an explicit `__all__` (including the lazy attributes) and basing `__dir__` on that list to make the public API more predictable and avoid potential duplicates.
## Individual Comments
### Comment 1
<location path="astrbot/core/__init__.py" line_range="30-39" />
<code_context>
- astrbot_config.get("pip_install_arg", ""),
- astrbot_config.get("pypi_index_url", None),
-)
+_initialized = False
+
+
+def _bootstrap():
+ """
+ internal initialization function, triggered only on first access to global instances.
+ Do not call this function directly from outside.
+ """
+ global _initialized
+ if _initialized:
+ return
+ _initialized = True
+
+ global \
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider making `_bootstrap` thread-safe to avoid double initialization under concurrent access.
Since `_bootstrap` is lazily invoked via `__getattr__`, multiple threads can enter it at the same time. With the current `if _initialized: return` check and no synchronization, two threads can both see `_initialized == False` and run the initialization twice, causing duplicate side effects (e.g., repeated logger setup or helper re-instantiation). Wrapping the initialization section in a `threading.Lock` (or similar) would ensure it runs exactly once while preserving lazy initialization.
</issue_to_address>
### Comment 2
<location path="astrbot/core/__init__.py" line_range="88-92" />
<code_context>
+}
+
+
+def __getattr__(name: str):
+ """lazy load global instances on first access"""
+ if name in _lazy_attrs:
+ _bootstrap()
+ return globals()[name]
+ raise AttributeError(f"module 'astrbot.core' has no attribute {name!r}")
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid leaking `KeyError` from `__getattr__` and always raise `AttributeError` for missing/failed lazy attributes.
If `_bootstrap` fails or a lazy attribute is never defined, `globals()[name]` will raise `KeyError`, which is unexpected from `__getattr__` (callers and tools expect `AttributeError` when an attribute is unavailable). Consider using `globals().get(name)` and explicitly raising `AttributeError` when the key is missing so all failure cases consistently surface as `AttributeError`.
</issue_to_address>
### Comment 3
<location path="astrbot/core/__init__.py" line_range="76" />
<code_context>
+
+
+# PEP 562 module-level __getattr__ and __dir__ for lazy loading of global instances
+_lazy_attrs = {
+ "astrbot_config",
+ "t2i_base_url",
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing the definition of lazy-loaded globals into a single source of truth and reusing it in both `_bootstrap` and the PEP 562 hooks to reduce coupling and maintenance overhead.
One concrete way to reduce the “magic” while keeping full backwards‑compatible lazy behavior is to centralize the lazy bookkeeping and avoid the separate `_lazy_attrs` / `__dir__` coupling.
Right now you have to keep these three things in sync:
- The globals declared in `_bootstrap`
- The `_lazy_attrs` set
- The behavior of `__dir__`
You can make this more maintainable by defining the lazy names in a single place and reusing that in both `_bootstrap` and the PEP 562 hooks.
Example refactor (focus on the pattern, not exact naming):
```python
DEMO_MODE = os.getenv("DEMO_MODE", "False").strip().lower() in ("true", "1", "t")
# single source of truth for lazy globals
_LAZY_GLOBAL_NAMES = (
"astrbot_config",
"t2i_base_url",
"html_renderer",
"logger",
"db_helper",
"sp",
"file_token_service",
"pip_installer",
)
_initialized = False
def _bootstrap() -> None:
"""Initialize global instances on first use. Internal only."""
global _initialized
if _initialized:
return
_initialized = True
# explicit globals, but derived from the single list above
global astrbot_config, t2i_base_url, html_renderer
global logger, db_helper, sp, file_token_service, pip_installer
os.makedirs(get_astrbot_data_path(), exist_ok=True)
astrbot_config = AstrBotConfig()
t2i_base_url = astrbot_config.get(
"t2i_endpoint", "https://t2i.soulter.top/text2img"
)
html_renderer = HtmlRenderer(t2i_base_url)
logger = LogManager.GetLogger(log_name="astrbot")
LogManager.configure_logger(logger, astrbot_config)
LogManager.configure_trace_logger(astrbot_config)
db_helper = SQLiteDatabase(DB_PATH)
sp = SharedPreferences(db_helper=db_helper)
file_token_service = FileTokenService()
pip_installer = PipInstaller(
astrbot_config.get("pip_install_arg", ""),
astrbot_config.get("pypi_index_url", None),
)
def __getattr__(name: str):
"""Lazy-load global instances on first attribute access."""
if name in _LAZY_GLOBAL_NAMES:
_bootstrap()
return globals()[name]
raise AttributeError(f"module 'astrbot.core' has no attribute {name!r}")
def __dir__():
"""Expose lazy-loaded attributes for dir() and IDEs."""
public_api = [k for k in globals() if not k.startswith("_")]
return sorted(set(public_api) | set(_LAZY_GLOBAL_NAMES))
```
Benefits while preserving behavior:
- Only one list (`_LAZY_GLOBAL_NAMES`) needs updating when adding/removing a lazy global.
- `__getattr__` and `__dir__` both derive from that list, removing the risk of them drifting apart.
- Control flow is easier to follow: the “lazy surface area” is explicitly listed and visible at the top of the module.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request refactors astrbot/core/__init__.py to implement lazy loading of global instances using PEP 562 module-level __getattr__ and __dir__, moving the initialization logic into a deferred _bootstrap() function. The review feedback highlights three key improvements: first, delay setting _initialized = True to the end of _bootstrap() to ensure robustness if initialization fails; second, guard against KeyError in __getattr__ to prevent crashing hasattr(); and third, return a sorted set of unique names in __dir__ to avoid duplicate entries and improve IDE auto-completion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
… sorted set of unique names in __dir__;
…ng for initialization
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces lazy loading for global instances in astrbot/core/__init__.py using PEP 562 __getattr__ and __dir__ with a thread-safe bootstrap mechanism. The reviewer identified a potential concurrency issue in __dir__() where iterating over globals() while another thread is initializing globals could cause a RuntimeError: dictionary changed size during iteration. The reviewer suggested wrapping the globals() iteration inside the bootstrap lock to prevent this issue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors astrbot/core/__init__.py to introduce lazy loading of global instances using PEP 562 module-level __getattr__ and __dir__ with thread-safe double-checked locking. The review feedback suggests adding TYPE_CHECKING imports and type annotations for these lazy-loaded global variables to maintain IDE auto-completion and static type checking.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
… for better organization
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces lazy loading for global instances in astrbot/core/__init__.py using PEP 562 module-level __getattr__ and __dir__ to defer initialization until they are first accessed. The reviewer suggested initializing all instances into local variables first before binding them to the global namespace to prevent partial initialization states if an exception occurs. Additionally, since the initialization runs synchronously in a single-threaded asyncio event loop, the thread locks can be safely removed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors astrbot/core/__init__.py to implement lazy loading of global instances (such as astrbot_config, html_renderer, logger, and others) using PEP 562 module-level __getattr__ and __dir__. This defers the initialization of these components until they are first accessed. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
… global instances
fix #8953,为core模块的__init__.py添加懒加载机制
Modifications / 改动点
对
astrbot/core/__init__.py进行修改,副作用代码改为懒加载Screenshots or Test Results / 运行截图或测试结果
导入core库,不在创建多余data目录
本地通过

make pr-test-full测试Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Refactor the core module’s initialization to lazily create and configure global instances only on first access, reducing side effects at import time.
Bug Fixes:
Enhancements: