sadpandajoe commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r3929779340
##########
tests/unit_tests/db_engine_specs/test_parseable.py:
##########
@@ -15,46 +15,36 @@
# specific language governing permissions and limitations
# under the License.
from datetime import datetime
-from typing import Optional
import pytest
+from superset.constants import TimeGrain
+from superset.db_engine_specs.parseable import ParseableEngineSpec
from tests.unit_tests.db_engine_specs.utils import assert_convert_dttm
from tests.unit_tests.fixtures.common import dttm # noqa: F401
-def test_epoch_to_dttm() -> None:
- """
- DB Eng Specs (parseable): Test epoch to dttm
- """
- from superset.db_engine_specs.parseable import ParseableEngineSpec
-
- assert ParseableEngineSpec.epoch_to_dttm() == "to_timestamp({col})"
-
+def test_parseable_properties() -> None:
+ assert ParseableEngineSpec.engine == "parseable"
+ assert ParseableEngineSpec.engine_name == "Parseable"
-def test_epoch_ms_to_dttm() -> None:
- """
- DB Eng Specs (parseable): Test epoch ms to dttm
- """
- from superset.db_engine_specs.parseable import ParseableEngineSpec
- assert ParseableEngineSpec.epoch_ms_to_dttm() == "to_timestamp({col} /
1000)"
+def test_parseable_metadata() -> None:
Review Comment:
This metadata rewrite removes the only check that `p_timestamp` receives the
`epoch_ms` and datetime handling that `alter_new_orm_column` still implements.
Could that runtime case remain covered alongside these metadata assertions?
##########
tests/unit_tests/db_engine_specs/test_ocient.py:
##########
@@ -14,404 +14,55 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
-# pylint: disable=import-outside-toplevel
-
-from typing import Any, Callable
-
import pytest
-from superset.db_engine_specs.ocient import (
- _point_list_to_wkt,
- _sanitized_ocient_type_codes,
-)
-from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-
-
-def ocient_is_installed() -> bool:
- return len(_sanitized_ocient_type_codes) > 0
-
+from superset.constants import TimeGrain
+from superset.db_engine_specs.ocient import OcientEngineSpec
-# (msg,expected)
-MARSHALED_OCIENT_ERRORS: list[tuple[str, SupersetError]] = [
- (
- "The referenced user does not exist (User 'mj' not found)",
- SupersetError(
- message='The username "mj" does not exist.',
- error_type=SupersetErrorType.CONNECTION_INVALID_USERNAME_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1012,
- "message": "Issue 1012 - The username provided when
connecting to a database is not valid.", # noqa: E501
- }
- ],
- },
- ),
- ),
- (
- "The userid/password combination was not valid (Incorrect password for
user)",
- SupersetError(
- message="The user/password combination is not valid (Incorrect
password for user).", # noqa: E501
- error_type=SupersetErrorType.CONNECTION_INVALID_PASSWORD_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1013,
- "message": "Issue 1013 - The password provided when
connecting to a database is not valid.", # noqa: E501
- }
- ],
- },
- ),
- ),
- (
- "No database named 'bulls' exists",
- SupersetError(
- message='Could not connect to database: "bulls"',
- error_type=SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1015,
- "message": "Issue 1015 - Either the database is
spelled incorrectly or does not exist.", # noqa: E501
- }
- ],
- },
- ),
- ),
- (
- "Unable to connect to unitedcenter.com:4050",
- SupersetError(
- message='Could not resolve hostname: "unitedcenter.com".',
- error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1007,
- "message": "Issue 1007 - The hostname provided can't
be resolved.", # noqa: E501
- }
- ],
- },
- ),
- ),
- (
- "Port out of range 0-65535",
- SupersetError(
- message="Port out of range 0-65535",
- error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1034,
- "message": "Issue 1034 - The port number is invalid.",
- }
- ],
- },
- ),
- ),
- (
- "An invalid connection string attribute was specified (failed to
decrypt cipher text)", # noqa: E501
- SupersetError(
- message="Invalid Connection String: Expecting String of the form
'ocient://user:pass@host:port/database'.",
- error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1002,
- "message": "Issue 1002 - The database returned an
unexpected error.", # noqa: E501
- }
- ],
- },
- ),
- ),
- (
- "There is a syntax error in your statement (extraneous input 'foo bar
baz' expecting {<EOF>, 'trace', 'using'})", # noqa: E501
- SupersetError(
- message="Syntax Error: extraneous input \"foo bar baz\" expecting
\"{<EOF>, 'trace', 'using'}", # noqa: E501
- error_type=SupersetErrorType.SYNTAX_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1030,
- "message": "Issue 1030 - The query has a syntax
error.",
- }
- ],
- },
- ),
- ),
- (
- "There is a syntax error in your statement (mismatched input 'to'
expecting {<EOF>, 'trace', 'using'})", # noqa: E501
- SupersetError(
- message="Syntax Error: mismatched input \"to\" expecting \"{<EOF>,
'trace', 'using'}", # noqa: E501
- error_type=SupersetErrorType.SYNTAX_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1030,
- "message": "Issue 1030 - The query has a syntax
error.",
- }
- ],
- },
- ),
- ),
- (
- "The referenced table or view 'goats' does not exist",
- SupersetError(
- message='Table or View "goats" does not exist.',
- error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1003,
- "message": "Issue 1003 - There is a syntax error in
the SQL query. Perhaps there was a misspelling or a typo.", # noqa: E501
- },
- {
- "code": 1005,
- "message": "Issue 1005 - The table was deleted or
renamed in the database.", # noqa: E501
- },
- ],
- },
- ),
- ),
- (
- "The reference to column 'goats' is not valid",
- SupersetError(
- message='Invalid reference to column: "goats"',
- error_type=SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR,
- level=ErrorLevel.ERROR,
- extra={
- "engine_name": "Ocient",
- "issue_codes": [
- {
- "code": 1003,
- "message": "Issue 1003 - There is a syntax error in
the SQL query. Perhaps there was a misspelling or a typo.", # noqa: E501
- },
- {
- "code": 1004,
- "message": "Issue 1004 - The column was deleted or
renamed in the database.", # noqa: E501
- },
- ],
- },
- ),
- ),
-]
+def test_ocient_properties() -> None:
+ assert OcientEngineSpec.engine == "ocient"
+ assert OcientEngineSpec.engine_name == "Ocient"
+ assert OcientEngineSpec.force_column_alias_quotes is True
+ assert OcientEngineSpec.max_column_name_length == 30
+ assert OcientEngineSpec.cte_alias == "cte__"
[email protected]("msg,expected", MARSHALED_OCIENT_ERRORS)
-def test_connection_errors(msg: str, expected: SupersetError) -> None:
- from superset.db_engine_specs.ocient import OcientEngineSpec
- result = OcientEngineSpec.extract_errors(Exception(msg))
- assert result == [expected]
+def test_ocient_metadata() -> None:
Review Comment:
Replacing this suite with metadata checks removes the only assertions for
Ocient error mapping and geometry conversion, although those paths remain live.
Could the behavioral cases stay alongside the metadata test so a driver-error
or GIS regression still fails coverage?
--
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]
