Skip to content

fix(redisdb): correct kwargs None check in RedisDB.__init__#307

Open
zhouxinfei wants to merge 3 commits intoBoris-code:masterfrom
zhouxinfei:fix/redisdb-kwargs-bug
Open

fix(redisdb): correct kwargs None check in RedisDB.__init__#307
zhouxinfei wants to merge 3 commits intoBoris-code:masterfrom
zhouxinfei:fix/redisdb-kwargs-bug

Conversation

@zhouxinfei
Copy link

Description

Fixed the kwargs check in RedisDB.__init__() from if kwargs is None: to if not kwargs:.

Problem

The original code incorrectly checks if kwargs is None:, but kwargs is always a dictionary (even if empty) in Python, never None. This causes the fallback to setting.REDISDB_KWARGS to never trigger when RedisDB() is called without arguments.

Solution

Changed the condition to if not kwargs: which correctly checks for an empty dictionary.

Changes Made

  1. feapder/db/redisdb.py:
    • Line XX: Changed if kwargs is None: to if not kwargs:

@safe1ine
Copy link

safe1ine commented Feb 5, 2026

@MonkeyCode-AI review 一下

@MonkeyCode-AI
Copy link

MonkeyCode-AI 正在分析任务...

Copy link

@MonkeyCode-AI MonkeyCode-AI left a comment

Choose a reason for hiding this comment

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

我是 MonkeyCode AI 编程助手,你可以在 GitHub 仓库的 PR 中 at @MonkeyCode-AI 来呼唤我。

任务执行细节请参考: https://monkeycode-ai.com

代码审查结果

修复了 RedisDB.__init__ 中对空 kwargs 的判断,使未显式传入连接参数时能够正确回退到 setting.REDISDB_KWARGS,变更合理且低风险。

🚨 Critical ⚠️ Warning 💡 Suggestion
0 0 1

if service_name is None:
service_name = setting.REDISDB_SERVICE_NAME
if kwargs is None:
if not kwargs:

Choose a reason for hiding this comment

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

Tip

💡 if not kwargs 会覆盖调用方显式传入的空字典语义(若需要区分“未提供”与“提供但为空”)

当前签名使用 **kwargs,调用方无法传入 None,只能传入若干关键字参数;因此 kwargs 为空通常代表“未提供额外参数”,回退到 setting.REDISDB_KWARGS 是合理的。但如果未来希望支持 RedisDB(**{}) 这类“显式要求不使用默认 kwargs”的语义(尽管当前也等价于没传),if not kwargs 会强制回退到全局默认,无法表达“空覆盖”。这属于产品/接口语义选择问题,不一定要改。

建议: 保持当前实现即可;若要区分语义,可改用显式参数(例如 kwargs: dict | None = None)而不是 **kwargs,并在内部合并/覆盖。

Suggested change
if not kwargs:

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants