fix(core): correctly parse DEMO_MODE as boolean from env var.#5676
fix(core): correctly parse DEMO_MODE as boolean from env var.#5676Soulter merged 2 commits intoAstrBotDevs:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些整体反馈:
- 建议在调用
.lower()之前先去除空白字符(例如使用.strip().lower()),这样像DEMO_MODE=" true "这样的值也能被正确解析。 - 如果你打算解析更多类似布尔值的字符串(例如
yes/no、on/off),把这类逻辑集中到一个小的辅助函数中可能会更简洁(例如parse_bool_env("DEMO_MODE", default=False)),并在其他类似标志位上复用。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider stripping whitespace before the `.lower()` call (e.g., `.strip().lower()`) so values like `DEMO_MODE=" true "` are still parsed correctly.
- If you expect to parse more boolean-like values (e.g., `yes/no`, `on/off`), it might be cleaner to centralize this in a small helper (e.g., `parse_bool_env("DEMO_MODE", default=False)`) and reuse it for similar flags.
## Individual Comments
### Comment 1
<location path="astrbot/core/__init__.py" line_range="17" />
<code_context>
os.makedirs(get_astrbot_data_path(), exist_ok=True)
-DEMO_MODE = os.getenv("DEMO_MODE", False)
+DEMO_MODE = os.getenv("DEMO_MODE", "False").lower() in ("true", "1", "t")
astrbot_config = AstrBotConfig()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider normalizing/stripping the env var value before the truthiness check.
If `DEMO_MODE` is set with trailing spaces or newlines (e.g., from env files), the current check will treat it as `False` even when it visually looks like `true`. Adding `.strip()` before `.lower()` makes this more robust, e.g. `os.getenv("DEMO_MODE", "False").strip().lower() in ("true", "1", "t")`, while preserving the current semantics.
```suggestion
DEMO_MODE = os.getenv("DEMO_MODE", "False").strip().lower() in ("true", "1", "t")
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Consider stripping whitespace before the
.lower()call (e.g.,.strip().lower()) so values likeDEMO_MODE=" true "are still parsed correctly. - If you expect to parse more boolean-like values (e.g.,
yes/no,on/off), it might be cleaner to centralize this in a small helper (e.g.,parse_bool_env("DEMO_MODE", default=False)) and reuse it for similar flags.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider stripping whitespace before the `.lower()` call (e.g., `.strip().lower()`) so values like `DEMO_MODE=" true "` are still parsed correctly.
- If you expect to parse more boolean-like values (e.g., `yes/no`, `on/off`), it might be cleaner to centralize this in a small helper (e.g., `parse_bool_env("DEMO_MODE", default=False)`) and reuse it for similar flags.
## Individual Comments
### Comment 1
<location path="astrbot/core/__init__.py" line_range="17" />
<code_context>
os.makedirs(get_astrbot_data_path(), exist_ok=True)
-DEMO_MODE = os.getenv("DEMO_MODE", False)
+DEMO_MODE = os.getenv("DEMO_MODE", "False").lower() in ("true", "1", "t")
astrbot_config = AstrBotConfig()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider normalizing/stripping the env var value before the truthiness check.
If `DEMO_MODE` is set with trailing spaces or newlines (e.g., from env files), the current check will treat it as `False` even when it visually looks like `true`. Adding `.strip()` before `.lower()` makes this more robust, e.g. `os.getenv("DEMO_MODE", "False").strip().lower() in ("true", "1", "t")`, while preserving the current semantics.
```suggestion
DEMO_MODE = os.getenv("DEMO_MODE", "False").strip().lower() in ("true", "1", "t")
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
fix(core): 添加.strip()以确保代码健壮性 Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request addresses an issue with parsing the DEMO_MODE environment variable, where string values like "False" were evaluated as truthy. The new implementation correctly converts the environment variable to a boolean. I have provided one suggestion to further improve the robustness of this boolean parsing by including more commonly used truthy string values.
Dt8333
left a comment
There was a problem hiding this comment.
问题的确存在。解决方案没问题。
(不过……应该没人设置DEMO_MODE为非TRUE的值吧。为什么不直接删了呢?)
感觉Gemini提供的修改意见似乎也没问题。(虽说疑似过度全面了)
|
No docs changes were generated in this run (docs repo had no updates). Docs repo: AstrBotDevs/AstrBot-docs AI change summary (not committed):
Experimental bot notice:
|
Fixes incorrect
DEMO_MODEevaluation when it is configured via environment variables.Previously, values like
DEMO_MODE=Falsemay be treated as truthy inif DEMO_MODE:checks (because non-empty strings are truthy in Python), causing demo-only logic to run unexpectedly. This change normalizesDEMO_MODEto a real boolean.Modifications / 改动点
astrbot/core/__init__.py: ParseDEMO_MODEfrom env var into a boolean (e.g., only"true","1","t"enable it;otherwise it is
Falseby default).This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
DEMO_MODE=Falseand confirm it is evaluated asFalse:$env:DEMO_MODE="False"uv run python -c "from astrbot.core import DEMO_MODE; print('DEMO_MODE=', DEMO_MODE); assert DEMO_MODE is False"DEMO_MODE=Trueand confirm it is evaluated asTrue:$env:DEMO_MODE="True"uv run python -c "from astrbot.core import DEMO_MODE; print('DEMO_MODE=', DEMO_MODE); assert DEMO_MODE is True"但是在该 debug 之前,就会导致被 Assert 掉
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.Summary by Sourcery
Bug Fixes:
DEMO_MODE真值判断不正确的问题,避免常见的“类似 false 的字符串”被错误地视为已启用。Original summary in English
Summary by Sourcery
Bug Fixes: