Skip to content

feat: DatasetSqlArgs添加engine_args属性#445

Merged
shijinpjlab merged 2 commits into
MigoXLab:devfrom
shijinpjlab:dev_0706
Jul 6, 2026
Merged

feat: DatasetSqlArgs添加engine_args属性#445
shijinpjlab merged 2 commits into
MigoXLab:devfrom
shijinpjlab:dev_0706

Conversation

@shijinpjlab

Copy link
Copy Markdown
Collaborator

No description provided.

@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 the engine_args parameter to DatasetSqlArgs and refactors SqlDataSource to safely construct connection URLs using SQLAlchemy's URL.create(...). It also implements robust parsing and validation for connection and engine arguments, updates the documentation, and adds comprehensive unit tests. The review feedback suggests improving the robustness of query argument parsing by stripping whitespace from keys and values, and using pytest.importorskip("pymysql") in MySQL-specific tests to prevent failures in environments where the driver is not installed.

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 on lines +79 to +83
return {
key: value
for key, value in parse_qsl(normalized, keep_blank_values=False)
if key
}

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.

medium

_parse_query_arg_string 中,通过 parse_qsl 解析出的查询参数键和值可能包含首尾空格(例如用户输入了 pool_recycle = 1800)。这会导致键查找失败或解析错误。对键和值进行 strip() 处理可以使解析更加健壮。

Suggested change
return {
key: value
for key, value in parse_qsl(normalized, keep_blank_values=False)
if key
}
return {
key.strip(): value.strip()
for key, value in parse_qsl(normalized, keep_blank_values=False)
if key.strip()
}

Comment on lines +246 to +247
def test_mysql_engine_has_stability_pool_settings():
sql_config = DatasetSqlArgs(

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.

medium

测试 test_mysql_engine_has_stability_pool_settings 尝试使用 pymysql 驱动创建 MySQL 引擎。如果测试环境中未安装 pymysql,该测试将因 ModuleNotFoundErrorNoSuchModuleError 而失败。在测试函数开头使用 pytest.importorskip("pymysql") 可以确保在缺少该驱动时优雅地跳过此测试。

Suggested change
def test_mysql_engine_has_stability_pool_settings():
sql_config = DatasetSqlArgs(
def test_mysql_engine_has_stability_pool_settings():
pytest.importorskip("pymysql")
sql_config = DatasetSqlArgs(

Comment on lines +294 to +295
def test_engine_args_override_default_pool_recycle():
sql_config = DatasetSqlArgs(

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.

medium

测试 test_engine_args_override_default_pool_recycle 尝试使用 pymysql 驱动创建 MySQL 引擎。如果测试环境中未安装 pymysql,该测试将因 ModuleNotFoundErrorNoSuchModuleError 而失败。在测试函数开头使用 pytest.importorskip("pymysql") 可以确保在缺少该驱动时优雅地跳过此测试。

Suggested change
def test_engine_args_override_default_pool_recycle():
sql_config = DatasetSqlArgs(
def test_engine_args_override_default_pool_recycle():
pytest.importorskip("pymysql")
sql_config = DatasetSqlArgs(

@shijinpjlab shijinpjlab merged commit b14f7c7 into MigoXLab:dev Jul 6, 2026
2 checks passed
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.

2 participants