Skip to content

refactor(core): restructure initialization logic in __init__.py for…#8954

Open
Initial01LingXu wants to merge 6 commits into
AstrBotDevs:masterfrom
Initial01LingXu:refactor(core)/core-remove-import-side-effects
Open

refactor(core): restructure initialization logic in __init__.py for…#8954
Initial01LingXu wants to merge 6 commits into
AstrBotDevs:masterfrom
Initial01LingXu:refactor(core)/core-remove-import-side-effects

Conversation

@Initial01LingXu

@Initial01LingXu Initial01LingXu commented Jun 22, 2026

Copy link
Copy Markdown

… global instances

fix #8953,为core模块的__init__.py添加懒加载机制

Modifications / 改动点

astrbot/core/__init__.py进行修改,副作用代码改为懒加载

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

导入core库,不在创建多余data目录

本地通过make pr-test-full测试
image


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.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.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:

  • Avoid creating the data directory and other side effects when merely importing the core module by deferring initialization until first attribute access.

Enhancements:

  • Introduce an internal bootstrap routine and PEP 562-based lazy attribute access in astrbot.core to centralize and defer setup of configuration, logging, database, and related services.

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 22, 2026
@vercel

vercel Bot commented Jun 22, 2026

Copy link
Copy Markdown

@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.

@dosubot dosubot Bot added the area:core The bug / feature is about astrbot's core, backend label Jun 22, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've found 3 issues, and left some high level feedback:

  • 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/core/__init__.py
Comment thread astrbot/core/__init__.py
Comment thread astrbot/core/__init__.py Outdated

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread astrbot/core/__init__.py
Comment thread astrbot/core/__init__.py
Comment thread astrbot/core/__init__.py Outdated
@Initial01LingXu

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread astrbot/core/__init__.py
@Initial01LingXu

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread astrbot/core/__init__.py Outdated
Comment thread astrbot/core/__init__.py Outdated
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 22, 2026
@Initial01LingXu

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread astrbot/core/__init__.py Outdated
@Initial01LingXu

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] core模块__init__.py存在副作用代码

1 participant