codeant-ai-for-open-source[bot] commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r3890461950
##########
tests/unit_tests/db_engine_specs/test_odps.py:
##########
@@ -14,161 +14,19 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-import logging
-from unittest.mock import MagicMock, patch
-
-import pytest
-from sqlalchemy.dialects import sqlite
-
-from superset.daos.database import DatabaseDAO
from superset.db_engine_specs.odps import OdpsBaseEngineSpec, OdpsEngineSpec
-from superset.sql.parse import Partition, Table
-
-
-def test_odps_base_engine_spec_get_table_metadata_raises() -> None:
- """OdpsBaseEngineSpec.get_table_metadata must not be called directly."""
- with pytest.raises(NotImplementedError):
- OdpsBaseEngineSpec.get_table_metadata(
- database=MagicMock(),
- table=Table("my_table", None, None),
- )
-
-
-def test_odps_engine_spec_select_star_no_partition() -> None:
- """select_star for a non-partitioned ODPS table produces a plain SELECT
*."""
- database = MagicMock()
- database.backend = "odps"
- database.get_columns.return_value = []
- database.compile_sqla_query = lambda query, catalog, schema: str(
- query.compile(dialect=sqlite.dialect())
- )
- dialect = sqlite.dialect()
-
- sql = OdpsEngineSpec.select_star(
- database=database,
- table=Table("my_table", None, None),
- dialect=dialect,
- limit=100,
- show_cols=False,
- indent=False,
- latest_partition=False,
- partition=None,
- )
-
- assert "SELECT" in sql
- assert "my_table" in sql
-
-
-def test_odps_engine_spec_select_star_with_partition() -> None:
- """select_star for a partitioned ODPS table adds a WHERE clause."""
- database = MagicMock()
- database.backend = "odps"
- database.get_columns.return_value = []
- database.compile_sqla_query = lambda query, catalog, schema: str(
- query.compile(dialect=sqlite.dialect())
- )
- dialect = sqlite.dialect()
- partition = Partition(is_partitioned_table=True,
partition_column=("month",))
-
- sql = OdpsEngineSpec.select_star(
- database=database,
- table=Table("my_table", None, None),
- dialect=dialect,
- limit=100,
- show_cols=False,
- indent=False,
- latest_partition=False,
- partition=partition,
- )
-
- assert "WHERE" in sql
-
-
-def test_is_odps_partitioned_table_non_odps_backend() -> None:
- """Returns (False, []) immediately for non-ODPS databases; no network call
made."""
- database = MagicMock()
- database.backend = "postgresql"
-
- result = DatabaseDAO.is_odps_partitioned_table(database, "some_table")
-
- assert result == (False, [])
-
-
-def test_is_odps_partitioned_table_missing_pyodps() -> None:
- """Returns (False, []) with a warning when pyodps is not installed."""
- database = MagicMock()
- database.backend = "odps"
- database.sqlalchemy_uri = (
- "odps://mykey:mysecret@myproject/?endpoint=http://service.odps.test";
- )
- database.password = "mysecret" # noqa: S105
-
- with patch("superset.daos.database.ODPS", None):
- result = DatabaseDAO.is_odps_partitioned_table(database, "some_table")
-
- assert result == (False, [])
-
-
-def test_is_odps_partitioned_table_uri_no_match(
- caplog: pytest.LogCaptureFixture,
-) -> None:
- """Logs a warning and returns (False, []) when the URI doesn't match the
pattern."""
- database = MagicMock()
- database.backend = "odps"
- database.sqlalchemy_uri = "odps://invalid-uri-format"
- database.password = "secret" # noqa: S105
-
- with patch("superset.daos.database.ODPS", MagicMock()):
- with caplog.at_level(logging.WARNING, logger="superset.daos.database"):
- result = DatabaseDAO.is_odps_partitioned_table(database,
"some_table")
-
- assert result == (False, [])
- assert "did not match" in caplog.text
-
-
-def test_is_odps_partitioned_table_partitioned(monkeypatch:
pytest.MonkeyPatch) -> None:
- """Returns (True, [field_names]) for a partitioned ODPS table."""
- database = MagicMock()
- database.backend = "odps"
- database.sqlalchemy_uri = (
- "odps://mykey:mysecret@myproject/?endpoint=http://service.odps.test";
- )
- database.password = "mysecret" # noqa: S105
-
- mock_partition = MagicMock()
- mock_partition.name = "month"
- mock_table = MagicMock()
- mock_table.exist_partition = True
- mock_table.table_schema.partitions = [mock_partition]
-
- mock_odps_client = MagicMock()
- mock_odps_client.get_table.return_value = mock_table
- mock_odps_class = MagicMock(return_value=mock_odps_client)
-
- with patch("superset.daos.database.ODPS", mock_odps_class):
- result = DatabaseDAO.is_odps_partitioned_table(database, "my_table")
-
- assert result == (True, ["month"])
-
-def test_is_odps_partitioned_table_not_partitioned(
- monkeypatch: pytest.MonkeyPatch,
-) -> None:
- """Returns (False, []) for a non-partitioned ODPS table."""
- database = MagicMock()
- database.backend = "odps"
- database.sqlalchemy_uri = (
- "odps://mykey:mysecret@myproject/?endpoint=http://service.odps.test";
- )
- database.password = "mysecret" # noqa: S105
- mock_table = MagicMock()
- mock_table.exist_partition = False
- mock_odps_client = MagicMock()
- mock_odps_client.get_table.return_value = mock_table
- mock_odps_class = MagicMock(return_value=mock_odps_client)
+def test_odps_properties() -> None:
+ assert OdpsEngineSpec.engine == "odps"
+ assert OdpsEngineSpec.engine_name == "ODPS (MaxCompute)"
+ assert OdpsEngineSpec.default_driver == "odps"
+ assert issubclass(OdpsEngineSpec, OdpsBaseEngineSpec)
Review Comment:
**Suggestion:** The replacement removes existing ODPS metadata behavior
tests for table columns, keys, indexes, partitions, and generated queries,
allowing regressions in those contracts to pass unnoticed. [incomplete
implementation]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8bd7952c3b964ac593ea42ddf5e878c7&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=8bd7952c3b964ac593ea42ddf5e878c7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/db_engine_specs/test_odps.py
**Line:** 20:24
**Comment:**
*Incomplete Implementation: The replacement removes existing ODPS
metadata behavior tests for table columns, keys, indexes, partitions, and
generated queries, allowing regressions in those contracts to pass unnoticed.
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%2F43695&comment_hash=cffde820166946202683963168fe251332c09fee4824b8841c6fd08a16602b66&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43695&comment_hash=cffde820166946202683963168fe251332c09fee4824b8841c6fd08a16602b66&reaction=dislike'>๐</a>
##########
superset/db_engine_specs/aurora.py:
##########
@@ -55,6 +79,34 @@ class AuroraPostgresDataAPI(PostgresEngineSpec):
"region_name={region_name}"
)
+ metadata = {
+ "description": (
+ "Amazon Aurora PostgreSQL via the Data API for serverless
connectivity."
+ ),
+ "logo": "aws-aurora.jpg",
+ "homepage_url": "https://aws.amazon.com/rds/aurora/";,
+ "categories": [
+ DatabaseCategory.CLOUD_AWS,
+ DatabaseCategory.HOSTED_OPEN_SOURCE,
+ ],
+ "pypi_packages": ["sqlalchemy-aurora-data-api"],
+ "connection_string": (
+
"postgresql+auroradataapi://{aws_access_id}:{aws_secret_access_key}@/"
+ "{database_name}?aurora_cluster_arn={aurora_cluster_arn}&"
+ "secret_arn={secret_arn}®ion_name={region_name}"
+ ),
+ "parameters": {
+ "aws_access_id": "AWS Access Key ID",
+ "aws_secret_access_key": "AWS Secret Access Key",
+ "database_name": "Database name",
+ "aurora_cluster_arn": "Aurora cluster ARN",
+ "secret_arn": "Secrets Manager ARN for credentials",
+ "region_name": "AWS region (e.g., us-east-1)",
+ },
+ "docs_url":
"https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/";,
+ "known_incompatibilities": AURORA_DATA_API_KNOWN_INCOMPATIBILITIES,
+ }
Review Comment:
**Suggestion:** `AuroraPostgresDataAPI` inherits
`PostgresEngineSpec.build_sqlalchemy_uri`, which reads
`metadata["default_port"]` for a missing port, but this metadata has no
`default_port`, causing a `KeyError`. [api mismatch]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Often`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=719ad7b66cc44522a70c7d8c658807b6&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=719ad7b66cc44522a70c7d8c658807b6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/aurora.py
**Line:** 82:108
**Comment:**
*Api Mismatch: `AuroraPostgresDataAPI` inherits
`PostgresEngineSpec.build_sqlalchemy_uri`, which reads
`metadata["default_port"]` for a missing port, but this metadata has no
`default_port`, causing a `KeyError`.
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%2F43695&comment_hash=4cf2b38cf90635fc54c4db358df5627aca3a96f48501c1dc1e7711cb95661d6a&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43695&comment_hash=4cf2b38cf90635fc54c4db358df5627aca3a96f48501c1dc1e7711cb95661d6a&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]
