codeant-ai-for-open-source[bot] commented on code in PR #41917:
URL: https://github.com/apache/superset/pull/41917#discussion_r3555885830
##########
tests/integration_tests/datasets/commands_tests.py:
##########
@@ -279,8 +279,9 @@ def test_export_dataset_command_unicode_chars(self, mock_g)
-> None:
mock_g.user = security_manager.find_user("admin")
examples_db = get_example_database()
with examples_db.get_sqla_engine() as engine:
- engine.execute("DROP TABLE IF EXISTS 中文")
- engine.execute("CREATE TABLE 中文 AS SELECT 2 as col")
+ with engine.begin() as conn:
+ conn.execute("DROP TABLE IF EXISTS 中文")
+ conn.execute("CREATE TABLE 中文 AS SELECT 2 as col")
Review Comment:
**Suggestion:** `Connection.execute()` in SQLAlchemy 2.x does not accept raw
SQL strings, so these setup statements will raise `ObjectNotExecutableError` at
runtime and the test will fail before reaching the command assertions. Wrap the
SQL in `text(...)` (or use `exec_driver_sql`) for both statements. [api
mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ test_export_dataset_command_unicode_chars fails at setup.
- ⚠️ ExportDatasetsCommand unicode handling unvalidated in CI.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the integration test suite, including
`test_export_dataset_command_unicode_chars`
in `tests/integration_tests/datasets/commands_tests.py:18-55` (verified via
Read tool).
2. Inside this test, `examples_db = get_example_database()` is called and
`examples_db.get_sqla_engine()` creates an SQLAlchemy engine context at
`commands_tests.py:21-22`.
3. The inner context `with engine.begin() as conn:` at
`commands_tests.py:23` opens a
SQLAlchemy 2.x `Connection`, which then calls `conn.execute("DROP TABLE IF
EXISTS 中文")`
and `conn.execute("CREATE TABLE 中文 AS SELECT 2 as col")` at hunk lines
283-284 using plain
strings.
4. Under SQLAlchemy 2.x semantics, `Connection.execute()` requires an
executable object
(e.g. `text(...)`); passing a bare string raises `ObjectNotExecutableError`,
causing the
test to fail during setup before reaching the `ExportDatasetsCommand` run
and assertions
at `commands_tests.py:41-47`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=700dd2df241d4530887024ee0292dd92&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=700dd2df241d4530887024ee0292dd92&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/datasets/commands_tests.py
**Line:** 283:284
**Comment:**
*Api Mismatch: `Connection.execute()` in SQLAlchemy 2.x does not accept
raw SQL strings, so these setup statements will raise
`ObjectNotExecutableError` at runtime and the test will fail before reaching
the command assertions. Wrap the SQL in `text(...)` (or use `exec_driver_sql`)
for both statements.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41917&comment_hash=b7c8bcd3ceaf97d3dd3df1a4b4bf53ced2f9294717f7601056ee93a0d9adb7cf&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41917&comment_hash=b7c8bcd3ceaf97d3dd3df1a4b4bf53ced2f9294717f7601056ee93a0d9adb7cf&reaction=dislike'>👎</a>
##########
tests/integration_tests/datasets/commands_tests.py:
##########
@@ -307,7 +308,8 @@ def test_export_dataset_command_unicode_chars(self, mock_g)
-> None:
db.session.delete(example_dataset)
db.session.commit()
with examples_db.get_sqla_engine() as engine:
- engine.execute("DROP TABLE 中文")
+ with engine.begin() as conn:
+ conn.execute("DROP TABLE 中文")
Review Comment:
**Suggestion:** This cleanup statement also uses a raw string with
`Connection.execute()`, which can raise `ObjectNotExecutableError` and break
test teardown. Use an executable SQL object (for example `text(...)`) or
`exec_driver_sql` so teardown always runs. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Teardown in test_export_dataset_command_unicode_chars crashes.
- ⚠️ Example database retains temporary table 中文 between tests.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run `test_export_dataset_command_unicode_chars` in
`tests/integration_tests/datasets/commands_tests.py:18-55` as part of the
integration
tests (function location verified via Read tool).
2. After creating and exporting the dataset, the test performs cleanup:
`db.session.delete(example_dataset)` and `db.session.commit()` at
`commands_tests.py:49-50`.
3. The test then opens a new engine context with
`examples_db.get_sqla_engine()` and
`engine.begin()` at `commands_tests.py:51-53`, calling `conn.execute("DROP
TABLE 中文")` at
hunk line 312 with a plain SQL string.
4. In SQLAlchemy 2.x, `Connection.execute()` with a bare string raises
`ObjectNotExecutableError`, so the teardown block crashes, potentially
leaving the `中文`
table undeleted in the example database and causing the test to fail during
cleanup.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f9855f4cf1344dbdb4287940891a3b79&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f9855f4cf1344dbdb4287940891a3b79&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/datasets/commands_tests.py
**Line:** 312:312
**Comment:**
*Api Mismatch: This cleanup statement also uses a raw string with
`Connection.execute()`, which can raise `ObjectNotExecutableError` and break
test teardown. Use an executable SQL object (for example `text(...)`) or
`exec_driver_sql` so teardown always runs.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41917&comment_hash=6065b1638d2d369d58850df6042dbc75c67c425a80c7cc20304f927155357663&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41917&comment_hash=6065b1638d2d369d58850df6042dbc75c67c425a80c7cc20304f927155357663&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
