codeant-ai-for-open-source[bot] commented on code in PR #41492:
URL: https://github.com/apache/superset/pull/41492#discussion_r3572435160
##########
tests/integration_tests/databases/api_tests.py:
##########
@@ -3692,6 +3696,7 @@ def test_available(self, get_available_engine_specs):
"disable_ssh_tunneling": False,
"supports_oauth2": False,
"supports_schemas": True,
+ "identifier_quote": {"start": '"', "end": '"'},
Review Comment:
**Suggestion:** The expected quote characters for the MySQL engine are wrong
in this assertion. `MySQLEngineSpec` now advertises backticks for identifier
quoting, so asserting double quotes will break this test once engine metadata
is returned correctly. [logic error]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
β /api/v1/database/available/ test fails for MySQL.
β οΈ Identifier quote metadata inconsistent for MySQL engine.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. In `tests/integration_tests/databases/api_tests.py`, examine
`test_available` starting
at lines 3426-3435 where `get_available_engine_specs.return_value` includes
`MySQLEngineSpec: {"mysqlconnector", "mysqldb"}`.
2. Within the same test, see the expected JSON response for the MySQL entry
at lines
195-252 of the snippet; line 3699 sets `"identifier_quote": {"start": '"',
"end": '"'},`
inside the MySQL `"engine_information"` block.
3. Follow the request `self.client.get("api/v1/database/available/")` to
`DatabaseRestApi.available` in `superset/databases/api.py:1948-2027`, which
for each
`engine_spec` calls `engine_spec.get_public_information()` and includes its
result under
`"engine_information"` (line 20).
4. Inspect `MySQLEngineSpec` in `superset/db_engine_specs/mysql.py:5-16`,
which explicitly
sets `identifier_quote_start = "`" and `identifier_quote_end = "`"`
(backticks) at lines
10-12. `BaseEngineSpec.get_public_information` in
`superset/db_engine_specs/base.py:2600-2612` constructs `"identifier_quote":
{"start":
cls.identifier_quote_start, "end": cls.identifier_quote_end}`, so the APIβs
MySQL
`engine_information.identifier_quote` will use backticks. Because
`test_available` asserts
double quotes for MySQL at line 3699, the test will consistently fail. The
companion test
`test_available_no_default` (lines 36-42 in the second snippet) correctly
expects
backticks, confirming the mismatch.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f03fc1224b674c99a882796ec02cac31&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=f03fc1224b674c99a882796ec02cac31&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/databases/api_tests.py
**Line:** 3699:3699
**Comment:**
*Logic Error: The expected quote characters for the MySQL engine are
wrong in this assertion. `MySQLEngineSpec` now advertises backticks for
identifier quoting, so asserting double quotes will break this test once engine
metadata is returned correctly.
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%2F41492&comment_hash=453c589979b5d6228275228b81a5274833e561f76ce2c580213d9435662e5946&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41492&comment_hash=453c589979b5d6228275228b81a5274833e561f76ce2c580213d9435662e5946&reaction=dislike'>π</a>
##########
tests/integration_tests/databases/api_tests.py:
##########
@@ -3633,6 +3636,7 @@ def test_available(self, get_available_engine_specs):
"disable_ssh_tunneling": True,
"supports_oauth2": True,
"supports_schemas": True,
+ "identifier_quote": {"start": "`", "end": "`"},
Review Comment:
**Suggestion:** The expected identifier quote for `GSheetsEngineSpec` is
incorrect here. `GSheetsEngineSpec` inherits from
`ShillelaghEngineSpec`/`BaseEngineSpec` and does not override identifier
quoting, so it should still use ANSI double quotes. Keeping backticks will make
this integration test fail against the actual API response. [logic error]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
β /api/v1/database/available/ test fails for GSheets.
β οΈ Identifier quote metadata inconsistent for Google Sheets.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Open `tests/integration_tests/databases/api_tests.py` and locate
`test_available` at
lines 3426-3445 (from Grep output), where `get_available_engine_specs` is
patched to
return a mapping including `GSheetsEngineSpec: {"apsw"}`.
2. Note that within `test_available`, the test issues
`self.client.get("api/v1/database/available/")` and asserts the full JSON
response literal
under `"databases"` including the Google Sheets block at lines 148-193 of
the snippet,
where line 3639 sets `"identifier_quote": {"start": "`", "end": "`"},`.
3. Follow the call to `DatabaseRestApi.available` in
`superset/databases/api.py:1948-2027`, which iterates over
`get_available_engine_specs().items()` and builds each entryβs
`"engine_information"` via
`engine_spec.get_public_information()` (line 20).
4. Inspect `GSheetsEngineSpec` in
`superset/db_engine_specs/gsheets.py:5-15`, which
inherits from `ShillelaghEngineSpec`
(`superset/db_engine_specs/shillelagh.py:5-12`).
Neither `ShillelaghEngineSpec` nor `GSheetsEngineSpec` override
`identifier_quote_start`/`end`, so they inherit the defaults from
`BaseEngineSpec`
(`superset/db_engine_specs/base.py:538-7`) which set `identifier_quote_start
= '"'` and
`identifier_quote_end = '"'`. `BaseEngineSpec.get_public_information` (lines
2600-2612)
returns `"identifier_quote": {"start": cls.identifier_quote_start, "end":
cls.identifier_quote_end}`, so the actual API response for the `gsheets`
engine will
contain double quotes, not backticks, causing `test_available` to fail on
this assertion.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=40cddde9ca394e04914e9a58388fc7ee&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=40cddde9ca394e04914e9a58388fc7ee&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/databases/api_tests.py
**Line:** 3639:3639
**Comment:**
*Logic Error: The expected identifier quote for `GSheetsEngineSpec` is
incorrect here. `GSheetsEngineSpec` inherits from
`ShillelaghEngineSpec`/`BaseEngineSpec` and does not override identifier
quoting, so it should still use ANSI double quotes. Keeping backticks will make
this integration test fail against the actual API response.
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%2F41492&comment_hash=b9ee4ee9d798279d449ff9a1ccc6751eabfb13fbb6c5bf24feeffb394fef9fe9&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41492&comment_hash=b9ee4ee9d798279d449ff9a1ccc6751eabfb13fbb6c5bf24feeffb394fef9fe9&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]
