You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2024/04/16 18:24:43 UTC
(superset) branch new-extra-table-metadata-api updated (03cdceb601 -> 6074fedf1d)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a change to branch new-extra-table-metadata-api
in repository https://gitbox.apache.org/repos/asf/superset.git
discard 03cdceb601 feat(sip-95): new endpoint for extra table metadata
new 6074fedf1d feat(sip-95): new endpoint for extra table metadata
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (03cdceb601)
\
N -- N -- N refs/heads/new-extra-table-metadata-api (6074fedf1d)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
superset/db_engine_specs/base.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
(superset) 01/01: feat(sip-95): new endpoint for extra table metadata
Posted by be...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch new-extra-table-metadata-api
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 6074fedf1d30b2ab31e85c6a8240b36c5709f547
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Tue Apr 16 13:58:35 2024 -0400
feat(sip-95): new endpoint for extra table metadata
---
.../src/SqlLab/actions/sqlLab.test.js | 2 +-
.../SqlEditorLeftBar/SqlEditorLeftBar.test.tsx | 2 +-
.../components/TableElement/TableElement.test.tsx | 2 +-
.../DndColumnSelectControl/DndFilterSelect.tsx | 2 +-
.../FilterControl/AdhocFilterControl/index.jsx | 2 +-
superset-frontend/src/hooks/apiResources/tables.ts | 6 +-
superset/constants.py | 1 +
superset/databases/api.py | 113 +++++++++++++-
superset/databases/schemas.py | 24 +++
superset/db_engine_specs/README.md | 2 +-
superset/db_engine_specs/base.py | 21 ++-
superset/db_engine_specs/bigquery.py | 8 +-
superset/db_engine_specs/gsheets.py | 9 +-
superset/db_engine_specs/lib.py | 10 +-
superset/db_engine_specs/presto.py | 22 ++-
superset/db_engine_specs/trino.py | 21 +--
superset/errors.py | 1 +
superset/exceptions.py | 26 ++++
tests/integration_tests/databases/api_tests.py | 12 +-
.../db_engine_specs/bigquery_tests.py | 12 +-
.../db_engine_specs/presto_tests.py | 9 +-
tests/unit_tests/conftest.py | 1 +
tests/unit_tests/databases/api_test.py | 171 ++++++++++++++++++++-
tests/unit_tests/db_engine_specs/test_trino.py | 8 +-
24 files changed, 419 insertions(+), 68 deletions(-)
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
index 20fd53ad38..ecf2c4d7e2 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -511,7 +511,7 @@ describe('async actions', () => {
const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/';
fetchMock.get(getTableMetadataEndpoint, {});
const getExtraTableMetadataEndpoint =
- 'glob:**/api/v1/database/*/table_extra/*/*/';
+ 'glob:**/api/v1/database/*/table_metadata/extra/';
fetchMock.get(getExtraTableMetadataEndpoint, {});
let isFeatureEnabledMock;
diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx
index f89c842b15..f8c94468bf 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.tsx
@@ -67,7 +67,7 @@ beforeEach(() => {
columns: table.columns,
},
});
- fetchMock.get('glob:*/api/v1/database/*/table_extra/*/*', {
+ fetchMock.get('glob:*/api/v1/database/*/table_metadata/extra/', {
status: 200,
body: {},
});
diff --git a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
index d9ac1d1e6f..74fd58b798 100644
--- a/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
+++ b/superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
@@ -49,7 +49,7 @@ jest.mock(
);
const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/';
const getExtraTableMetadataEndpoint =
- 'glob:**/api/v1/database/*/table_extra/*/*/';
+ 'glob:**/api/v1/database/*/table_metadata/extra/';
const updateTableSchemaEndpoint = 'glob:*/tableschemaview/*/expanded';
beforeEach(() => {
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
index b479ce4c87..696861b289 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
@@ -181,7 +181,7 @@ const DndFilterSelect = (props: DndFilterSelectProps) => {
if (!isSqllabView && dbId && name && schema) {
SupersetClient.get({
- endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`,
+ endpoint: `/api/v1/database/${dbId}/table_metadata/extra/?name=${name}&schema=${schema}`,
})
.then(({ json }: { json: Record<string, any> }) => {
if (json?.partitions) {
diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx
index 7b6a3938fd..c68a97b4d7 100644
--- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx
+++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx
@@ -143,7 +143,7 @@ class AdhocFilterControl extends React.Component {
if (!isSqllabView && dbId && name && schema) {
SupersetClient.get({
- endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`,
+ endpoint: `/api/v1/database/${dbId}/table_metadata/extra/?name=${name}&schema=${schema}`,
})
.then(({ json }) => {
if (json && json.partitions) {
diff --git a/superset-frontend/src/hooks/apiResources/tables.ts b/superset-frontend/src/hooks/apiResources/tables.ts
index 66eedc00b5..57c2a64527 100644
--- a/superset-frontend/src/hooks/apiResources/tables.ts
+++ b/superset-frontend/src/hooks/apiResources/tables.ts
@@ -125,9 +125,9 @@ const tableApi = api.injectEndpoints({
FetchTableMetadataQueryParams
>({
query: ({ dbId, schema, table }) => ({
- endpoint: `/api/v1/database/${dbId}/table_extra/${encodeURIComponent(
- table,
- )}/${encodeURIComponent(schema)}/`,
+ endpoint: schema
+ ? `/api/v1/database/${dbId}/table_metadata/extra/?table=${table}&schema=${schema}`
+ : `/api/v1/database/${dbId}/table_metadata/extra/?table=${table}`,
transformResponse: ({ json }: JsonResponse) => json,
}),
}),
diff --git a/superset/constants.py b/superset/constants.py
index bf4e7717d5..e4d467bdd8 100644
--- a/superset/constants.py
+++ b/superset/constants.py
@@ -135,6 +135,7 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = {
"select_star": "read",
"table_metadata": "read",
"table_extra_metadata": "read",
+ "table_extra_metadata_deprecated": "read",
"test_connection": "write",
"validate_parameters": "write",
"favorite_status": "read",
diff --git a/superset/databases/api.py b/superset/databases/api.py
index 5eb9de90d4..0fc3aaca92 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -15,11 +15,14 @@
# specific language governing permissions and limitations
# under the License.
# pylint: disable=too-many-lines
+
+from __future__ import annotations
+
import json
import logging
from datetime import datetime, timedelta
from io import BytesIO
-from typing import Any, cast, Optional
+from typing import Any, cast
from zipfile import is_zipfile, ZipFile
from deprecation import deprecated
@@ -80,6 +83,7 @@ from superset.databases.schemas import (
get_export_ids_schema,
OAuth2ProviderResponseSchema,
openapi_spec_methods_override,
+ QualifiedTableSchema,
SchemasResponseSchema,
SelectStarResponseSchema,
TableExtraMetadataResponseSchema,
@@ -90,9 +94,18 @@ from superset.databases.schemas import (
from superset.databases.utils import get_table_metadata
from superset.db_engine_specs import get_available_engine_specs
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-from superset.exceptions import OAuth2Error, SupersetErrorsException, SupersetException
+from superset.exceptions import (
+ DatabaseNotFoundException,
+ InvalidPayloadSchemaError,
+ OAuth2Error,
+ SupersetErrorsException,
+ SupersetException,
+ SupersetSecurityException,
+ TableNotFoundException,
+)
from superset.extensions import security_manager
from superset.models.core import Database
+from superset.sql_parse import Table
from superset.superset_typing import FlaskResponse
from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item
from superset.utils.oauth2 import decode_oauth2_state
@@ -119,6 +132,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"tables",
"table_metadata",
"table_extra_metadata",
+ "table_extra_metadata_deprecated",
"select_star",
"schemas",
"test_connection",
@@ -760,15 +774,20 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
@check_table_access
@safe
@statsd_metrics
+ @deprecated(deprecated_in="4.0", removed_in="5.0")
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
- f".table_extra_metadata",
+ f".table_extra_metadata_deprecated",
log_to_statsd=False,
)
- def table_extra_metadata(
+ def table_extra_metadata_deprecated(
self, database: Database, table_name: str, schema_name: str
) -> FlaskResponse:
"""Get table extra metadata.
+
+ A newer API was introduced between 4.0 and 5.0, with support for catalogs for
+ SIP-95. This method was kept to prevent breaking API integrations, but will be
+ removed in 5.0.
---
get:
summary: Get table extra metadata
@@ -812,9 +831,87 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
parsed_schema = parse_js_uri_path_item(schema_name, eval_undefined=True)
table_name = cast(str, parse_js_uri_path_item(table_name))
- payload = database.db_engine_spec.extra_table_metadata(
- database, table_name, parsed_schema
- )
+ table = Table(table_name, parsed_schema)
+ payload = database.db_engine_spec.get_extra_table_metadata(database, table)
+ return self.response(200, **payload)
+
+ @expose("/<int:pk>/table_metadata/extra/", methods=("GET",))
+ @protect()
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+ f".table_extra_metadata",
+ log_to_statsd=False,
+ )
+ def table_extra_metadata(self, pk: int) -> FlaskResponse:
+ """
+ Get extra metadata for a given table.
+
+ Optionally, a schema and a catalog can be passed, if different from the default
+ ones.
+ ---
+ get:
+ summary: Get table extra metadata
+ description: >-
+ Response depends on each DB engine spec normally focused on partitions.
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The database id
+ - in: query
+ schema:
+ type: string
+ name: table
+ required: true
+ description: Table name
+ - in: query
+ schema:
+ type: string
+ name: schema
+ description: >-
+ Optional table schema, if not passed default schema will be used
+ - in: query
+ schema:
+ type: string
+ name: catalog
+ description: >-
+ Optional table catalog, if not passed default catalog will be used
+ responses:
+ 200:
+ description: Table extra metadata information
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/TableExtraMetadataResponseSchema"
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ self.incr_stats("init", self.table_metadata.__name__)
+
+ database = DatabaseDAO.find_by_id(pk)
+ if database is None:
+ raise DatabaseNotFoundException("No such database")
+
+ try:
+ parameters = QualifiedTableSchema().load(request.args)
+ except ValidationError as ex:
+ raise InvalidPayloadSchemaError(ex) from ex
+
+ table = Table(**parameters)
+ try:
+ security_manager.raise_for_access(database=database, table=table)
+ except SupersetSecurityException as ex:
+ # instead of raising 403, raise 404 to hide table existence
+ raise TableNotFoundException("No such table") from ex
+
+ payload = database.db_engine_spec.get_extra_table_metadata(database, table)
+
return self.response(200, **payload)
@expose("/<int:pk>/select_star/<path:table_name>/", methods=("GET",))
@@ -828,7 +925,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
log_to_statsd=False,
)
def select_star(
- self, database: Database, table_name: str, schema_name: Optional[str] = None
+ self, database: Database, table_name: str, schema_name: str | None = None
) -> FlaskResponse:
"""Get database select star for table.
---
diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py
index 791cfd360e..1fe8e6b985 100644
--- a/superset/databases/schemas.py
+++ b/superset/databases/schemas.py
@@ -1013,3 +1013,27 @@ class OAuth2ProviderResponseSchema(Schema):
class Meta: # pylint: disable=too-few-public-methods
# Ignore unknown fields that might be sent by the OAuth2 provider
unknown = EXCLUDE
+
+
+class QualifiedTableSchema(Schema):
+ """
+ Schema for a qualified table reference.
+
+ Catalog and schema can be ommited, to fallback to default values. Table name must be
+ present.
+ """
+
+ table = fields.String(
+ required=True,
+ metadata={"description": "The table name"},
+ )
+ schema = fields.String(
+ required=False,
+ load_default=None,
+ metadata={"description": "The table schema"},
+ )
+ catalog = fields.String(
+ required=False,
+ load_default=None,
+ metadata={"description": "The table catalog"},
+ )
diff --git a/superset/db_engine_specs/README.md b/superset/db_engine_specs/README.md
index f158a3a41b..11f0f90ab7 100644
--- a/superset/db_engine_specs/README.md
+++ b/superset/db_engine_specs/README.md
@@ -603,7 +603,7 @@ For some databases the `df_to_sql` classmethod needs to be implemented. For exam
### Extra table metadata
-DB engine specs can return additional metadata associated with a table. This is done via the `extra_table_metadata` class method. Trino uses this to return information about the latest partition, for example, and Bigquery returns clustering information. This information is then surfaced in the SQL Lab UI, when browsing tables in the metadata explorer (on the left panel).
+DB engine specs can return additional metadata associated with a table. This is done via the `get_extra_table_metadata` class method. Trino uses this to return information about the latest partition, for example, and Bigquery returns clustering information. This information is then surfaced in the SQL Lab UI, when browsing tables in the metadata explorer (on the left panel).
### DB API exception mapping
diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index bcb4035c9c..6e35cce485 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -1034,7 +1034,23 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
return indexes
@classmethod
- def extra_table_metadata( # pylint: disable=unused-argument
+ def get_extra_table_metadata( # pylint: disable=unused-argument
+ cls,
+ database: Database,
+ table: Table,
+ ) -> dict[str, Any]:
+ """
+ Returns engine-specific table metadata
+
+ :param database: Database instance
+ :param table: A Table instance
+ :return: Engine-specific table metadata
+ """
+ return cls.extra_table_metadata(database, table.table, table.schema)
+
+ @deprecated(deprecated_in="4.0", removed_in="5.0")
+ @classmethod
+ def extra_table_metadata(
cls,
database: Database,
table_name: str,
@@ -1043,12 +1059,13 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
"""
Returns engine-specific table metadata
+ Deprecated, since it doesn't support catalogs.
+
:param database: Database instance
:param table_name: Table name
:param schema_name: Schema name
:return: Engine-specific table metadata
"""
- # TODO: Fix circular import caused by importing Database
return {}
@classmethod
diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py
index a8d834276e..7827531a79 100644
--- a/superset/db_engine_specs/bigquery.py
+++ b/superset/db_engine_specs/bigquery.py
@@ -320,10 +320,12 @@ class BigQueryEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-met
return cls.normalize_indexes(inspector.get_indexes(table_name, schema))
@classmethod
- def extra_table_metadata(
- cls, database: "Database", table_name: str, schema_name: Optional[str]
+ def get_extra_table_metadata(
+ cls,
+ database: "Database",
+ table: Table,
) -> dict[str, Any]:
- indexes = database.get_indexes(table_name, schema_name)
+ indexes = database.get_indexes(table.table, table.schema)
if not indexes:
return {}
partitions_columns = [
diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py
index 3e33aa8e32..4aed2693f4 100644
--- a/superset/db_engine_specs/gsheets.py
+++ b/superset/db_engine_specs/gsheets.py
@@ -137,15 +137,14 @@ class GSheetsEngineSpec(ShillelaghEngineSpec):
return url
@classmethod
- def extra_table_metadata(
+ def get_extra_table_metadata(
cls,
database: Database,
- table_name: str,
- schema_name: str | None,
+ table: Table,
) -> dict[str, Any]:
- with database.get_raw_connection(schema=schema_name) as conn:
+ with database.get_raw_connection(schema=table.schema) as conn:
cursor = conn.cursor()
- cursor.execute(f'SELECT GET_METADATA("{table_name}")')
+ cursor.execute(f'SELECT GET_METADATA("{table.table}")')
results = cursor.fetchone()[0]
try:
metadata = json.loads(results)
diff --git a/superset/db_engine_specs/lib.py b/superset/db_engine_specs/lib.py
index 4e9937c537..398d5af647 100644
--- a/superset/db_engine_specs/lib.py
+++ b/superset/db_engine_specs/lib.py
@@ -63,7 +63,7 @@ BASIC_FEATURES = {
NICE_TO_HAVE_FEATURES = {
"user_impersonation": "Supports user impersonation",
"file_upload": "Support file upload",
- "extra_table_metadata": "Returns extra table metadata",
+ "get_extra_table_metadata": "Returns extra table metadata",
"dbapi_exception_mapping": "Maps driver exceptions to Superset exceptions",
"custom_errors": "Parses error messages and returns Superset errors",
"dynamic_schema": "Supports changing the schema per-query",
@@ -142,7 +142,9 @@ def diagnose(spec: type[BaseEngineSpec]) -> dict[str, Any]:
or has_custom_method(spec, "get_url_for_impersonation")
),
"file_upload": spec.supports_file_upload,
- "extra_table_metadata": has_custom_method(spec, "extra_table_metadata"),
+ "get_extra_table_metadata": has_custom_method(
+ spec, "get_extra_table_metadata"
+ ),
"dbapi_exception_mapping": has_custom_method(
spec, "get_dbapi_exception_mapping"
),
@@ -177,7 +179,7 @@ def diagnose(spec: type[BaseEngineSpec]) -> dict[str, Any]:
nice_to_have = [
"user_impersonation",
"file_upload",
- "extra_table_metadata",
+ "get_extra_table_metadata",
"dbapi_exception_mapping",
"custom_errors",
"dynamic_schema",
@@ -264,7 +266,7 @@ def generate_table() -> list[list[Any]]:
keys = [
"user_impersonation",
"file_upload",
- "extra_table_metadata",
+ "get_extra_table_metadata",
"dbapi_exception_mapping",
"custom_errors",
"dynamic_schema",
diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py
index d749d2cb18..a8143c87ed 100644
--- a/superset/db_engine_specs/presto.py
+++ b/superset/db_engine_specs/presto.py
@@ -66,6 +66,7 @@ from superset.utils.core import GenericDataType
if TYPE_CHECKING:
# prevent circular imports
from superset.models.core import Database
+ from superset.sql_parse import Table
with contextlib.suppress(ImportError): # pyhive may not be installed
from pyhive.presto import Cursor
@@ -1224,14 +1225,20 @@ class PrestoEngineSpec(PrestoBaseEngineSpec):
return all_columns, data, expanded_columns
@classmethod
- def extra_table_metadata(
- cls, database: Database, table_name: str, schema_name: str | None
+ def get_extra_table_metadata(
+ cls,
+ database: Database,
+ table: Table,
) -> dict[str, Any]:
metadata = {}
- if indexes := database.get_indexes(table_name, schema_name):
+ if indexes := database.get_indexes(table.table, table.schema):
col_names, latest_parts = cls.latest_partition(
- table_name, schema_name, database, show_first=True, indexes=indexes
+ table.table,
+ table.schema,
+ database,
+ show_first=True,
+ indexes=indexes,
)
if not latest_parts:
@@ -1241,8 +1248,8 @@ class PrestoEngineSpec(PrestoBaseEngineSpec):
"cols": sorted(indexes[0].get("column_names", [])),
"latest": dict(zip(col_names, latest_parts)),
"partitionQuery": cls._partition_query(
- table_name=table_name,
- schema=schema_name,
+ table_name=table.table,
+ schema=table.schema,
indexes=indexes,
database=database,
),
@@ -1250,7 +1257,8 @@ class PrestoEngineSpec(PrestoBaseEngineSpec):
# flake8 is not matching `Optional[str]` to `Any` for some reason...
metadata["view"] = cast(
- Any, cls.get_create_view(database, schema_name, table_name)
+ Any,
+ cls.get_create_view(database, table.schema, table.table),
)
return metadata
diff --git a/superset/db_engine_specs/trino.py b/superset/db_engine_specs/trino.py
index 2185de8c86..32c119ec1f 100644
--- a/superset/db_engine_specs/trino.py
+++ b/superset/db_engine_specs/trino.py
@@ -45,6 +45,7 @@ from superset.utils import core as utils
if TYPE_CHECKING:
from superset.models.core import Database
+ from superset.sql_parse import Table
with contextlib.suppress(ImportError): # trino may not be installed
from trino.dbapi import Cursor
@@ -58,18 +59,17 @@ class TrinoEngineSpec(PrestoBaseEngineSpec):
allows_alias_to_source_column = False
@classmethod
- def extra_table_metadata(
+ def get_extra_table_metadata(
cls,
database: Database,
- table_name: str,
- schema_name: str | None,
+ table: Table,
) -> dict[str, Any]:
metadata = {}
- if indexes := database.get_indexes(table_name, schema_name):
+ if indexes := database.get_indexes(table.table, table.schema):
col_names, latest_parts = cls.latest_partition(
- table_name,
- schema_name,
+ table.table,
+ table.schema,
database,
show_first=True,
indexes=indexes,
@@ -91,17 +91,18 @@ class TrinoEngineSpec(PrestoBaseEngineSpec):
),
"latest": dict(zip(col_names, latest_parts)),
"partitionQuery": cls._partition_query(
- table_name=table_name,
- schema=schema_name,
+ table_name=table.table,
+ schema=table.schema,
indexes=indexes,
database=database,
),
}
- if database.has_view_by_name(table_name, schema_name):
+ if database.has_view_by_name(table.table, table.schema):
with database.get_inspector_with_context() as inspector:
metadata["view"] = inspector.get_view_definition(
- table_name, schema_name
+ table.table,
+ table.schema,
)
return metadata
diff --git a/superset/errors.py b/superset/errors.py
index 612977ad65..9f0f5bed05 100644
--- a/superset/errors.py
+++ b/superset/errors.py
@@ -73,6 +73,7 @@ class SupersetErrorType(StrEnum):
# Other errors
BACKEND_TIMEOUT_ERROR = "BACKEND_TIMEOUT_ERROR"
DATABASE_NOT_FOUND_ERROR = "DATABASE_NOT_FOUND_ERROR"
+ TABLE_NOT_FOUND_ERROR = "TABLE_NOT_FOUND_ERROR"
# Sql Lab errors
MISSING_TEMPLATE_PARAMS_ERROR = "MISSING_TEMPLATE_PARAMS_ERROR"
diff --git a/superset/exceptions.py b/superset/exceptions.py
index 45cbdf3c87..0315ee30f4 100644
--- a/superset/exceptions.py
+++ b/superset/exceptions.py
@@ -362,3 +362,29 @@ class CreateKeyValueDistributedLockFailedException(Exception):
"""
Exception to signalize failure to acquire lock.
"""
+
+
+class DatabaseNotFoundException(SupersetErrorException):
+ status = 404
+
+ def __init__(self, message: str):
+ super().__init__(
+ SupersetError(
+ message=message,
+ error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ )
+
+
+class TableNotFoundException(SupersetErrorException):
+ status = 404
+
+ def __init__(self, message: str):
+ super().__init__(
+ SupersetError(
+ message=message,
+ error_type=SupersetErrorType.TABLE_NOT_FOUND_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ )
diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py
index b944242259..eea616c33d 100644
--- a/tests/integration_tests/databases/api_tests.py
+++ b/tests/integration_tests/databases/api_tests.py
@@ -1502,9 +1502,9 @@ class TestDatabaseApi(SupersetTestCase):
self.assertEqual(rv.status_code, 404)
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
- def test_get_table_extra_metadata(self):
+ def test_get_table_extra_metadata_deprecated(self):
"""
- Database API: Test get table extra metadata info
+ Database API: Test deprecated get table extra metadata info
"""
example_db = get_example_database()
self.login(ADMIN_USERNAME)
@@ -1514,9 +1514,9 @@ class TestDatabaseApi(SupersetTestCase):
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(response, {})
- def test_get_invalid_database_table_extra_metadata(self):
+ def test_get_invalid_database_table_extra_metadata_deprecated(self):
"""
- Database API: Test get invalid database from table extra metadata
+ Database API: Test get invalid database from deprecated table extra metadata
"""
database_id = 1000
self.login(ADMIN_USERNAME)
@@ -1528,9 +1528,9 @@ class TestDatabaseApi(SupersetTestCase):
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)
- def test_get_invalid_table_table_extra_metadata(self):
+ def test_get_invalid_table_table_extra_metadata_deprecated(self):
"""
- Database API: Test get invalid table from table extra metadata
+ Database API: Test get invalid table from deprecated table extra metadata
"""
example_db = get_example_database()
uri = f"api/v1/database/{example_db.id}/table_extra/wrong_table/null/"
diff --git a/tests/integration_tests/db_engine_specs/bigquery_tests.py b/tests/integration_tests/db_engine_specs/bigquery_tests.py
index 3eaaa3d1c7..c012376b39 100644
--- a/tests/integration_tests/db_engine_specs/bigquery_tests.py
+++ b/tests/integration_tests/db_engine_specs/bigquery_tests.py
@@ -111,15 +111,16 @@ class TestBigQueryDbEngineSpec(TestDbEngineSpec):
result = BigQueryEngineSpec.fetch_data(None, 0)
self.assertEqual(result, [1, 2])
- def test_extra_table_metadata(self):
+ def test_get_extra_table_metadata(self):
"""
DB Eng Specs (bigquery): Test extra table metadata
"""
database = mock.Mock()
# Test no indexes
database.get_indexes = mock.MagicMock(return_value=None)
- result = BigQueryEngineSpec.extra_table_metadata(
- database, "some_table", "some_schema"
+ result = BigQueryEngineSpec.get_extra_table_metadata(
+ database,
+ Table("some_table", "some_schema"),
)
self.assertEqual(result, {})
@@ -138,8 +139,9 @@ class TestBigQueryDbEngineSpec(TestDbEngineSpec):
"clustering": {"cols": [["c_col1", "c_col2", "c_col3"]]},
}
database.get_indexes = mock.MagicMock(return_value=index_metadata)
- result = BigQueryEngineSpec.extra_table_metadata(
- database, "some_table", "some_schema"
+ result = BigQueryEngineSpec.get_extra_table_metadata(
+ database,
+ Table("some_table", "some_schema"),
)
self.assertEqual(result, expected_result)
diff --git a/tests/integration_tests/db_engine_specs/presto_tests.py b/tests/integration_tests/db_engine_specs/presto_tests.py
index c28e78afe6..81a9e61105 100644
--- a/tests/integration_tests/db_engine_specs/presto_tests.py
+++ b/tests/integration_tests/db_engine_specs/presto_tests.py
@@ -25,7 +25,7 @@ from sqlalchemy.sql import select
from superset.db_engine_specs.presto import PrestoEngineSpec
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-from superset.sql_parse import ParsedQuery
+from superset.sql_parse import ParsedQuery, Table
from superset.utils.database import get_example_database
from tests.integration_tests.db_engine_specs.base_tests import TestDbEngineSpec
@@ -549,7 +549,7 @@ class TestPrestoDbEngineSpec(TestDbEngineSpec):
self.assertEqual(actual_data, expected_data)
self.assertEqual(actual_expanded_cols, expected_expanded_cols)
- def test_presto_extra_table_metadata(self):
+ def test_presto_get_extra_table_metadata(self):
database = mock.Mock()
database.get_indexes = mock.Mock(
return_value=[{"column_names": ["ds", "hour"]}]
@@ -558,8 +558,9 @@ class TestPrestoDbEngineSpec(TestDbEngineSpec):
df = pd.DataFrame({"ds": ["01-01-19"], "hour": [1]})
database.get_df = mock.Mock(return_value=df)
PrestoEngineSpec.get_create_view = mock.Mock(return_value=None)
- result = PrestoEngineSpec.extra_table_metadata(
- database, "test_table", "test_schema"
+ result = PrestoEngineSpec.get_extra_table_metadata(
+ database,
+ Table("test_table", "test_schema"),
)
assert result["partitions"]["cols"] == ["ds", "hour"]
assert result["partitions"]["latest"] == {"ds": "01-01-19", "hour": 1}
diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py
index 3824d7b7b7..02873fcf40 100644
--- a/tests/unit_tests/conftest.py
+++ b/tests/unit_tests/conftest.py
@@ -146,6 +146,7 @@ def full_api_access(mocker: MockFixture) -> Iterator[None]:
"flask_appbuilder.security.decorators.verify_jwt_in_request",
return_value=True,
)
+ mocker.patch.object(security_manager, "is_item_public", return_value=True)
mocker.patch.object(security_manager, "has_access", return_value=True)
mocker.patch.object(security_manager, "can_access_all_databases", return_value=True)
diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py
index ff6dcb2334..03f090514f 100644
--- a/tests/unit_tests/databases/api_test.py
+++ b/tests/unit_tests/databases/api_test.py
@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.
-# pylint: disable=unused-argument, import-outside-toplevel, line-too-long
+# pylint: disable=unused-argument, import-outside-toplevel, line-too-long, invalid-name
import json
from datetime import datetime
@@ -32,6 +32,9 @@ from sqlalchemy.orm.session import Session
from superset import db
from superset.db_engine_specs.sqlite import SqliteEngineSpec
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import SupersetSecurityException
+from superset.sql_parse import Table
def test_filter_by_uuid(
@@ -792,8 +795,6 @@ def test_oauth2_multiple_tokens(
def test_oauth2_error(
- mocker: MockFixture,
- session: Session,
client: Any,
full_api_access: None,
) -> None:
@@ -818,3 +819,167 @@ def test_oauth2_error(
}
]
}
+
+
+def test_table_extra_metadata_happy_path(
+ mocker: MockFixture,
+ client: Any,
+ full_api_access: None,
+) -> None:
+ """
+ Test the `table_extra_metadata` endpoint.
+ """
+ database = mocker.MagicMock()
+ database.db_engine_spec.get_extra_table_metadata.return_value = {"hello": "world"}
+ mocker.patch("superset.databases.api.DatabaseDAO.find_by_id", return_value=database)
+ mocker.patch("superset.databases.api.security_manager.raise_for_access")
+
+ response = client.get("/api/v1/database/1/table_metadata/extra/?table=t")
+ assert response.json == {"hello": "world"}
+ database.db_engine_spec.get_extra_table_metadata.assert_called_with(
+ database,
+ Table("t"),
+ )
+
+ response = client.get("/api/v1/database/1/table_metadata/extra/?table=t&schema=s")
+ database.db_engine_spec.get_extra_table_metadata.assert_called_with(
+ database,
+ Table("t", "s"),
+ )
+
+ response = client.get("/api/v1/database/1/table_metadata/extra/?table=t&catalog=c")
+ database.db_engine_spec.get_extra_table_metadata.assert_called_with(
+ database,
+ Table("t", None, "c"),
+ )
+
+ response = client.get(
+ "/api/v1/database/1/table_metadata/extra/?table=t&schema=s&catalog=c"
+ )
+ database.db_engine_spec.get_extra_table_metadata.assert_called_with(
+ database,
+ Table("t", "s", "c"),
+ )
+
+
+def test_table_extra_metadata_no_table(
+ mocker: MockFixture,
+ client: Any,
+ full_api_access: None,
+) -> None:
+ """
+ Test the `table_extra_metadata` endpoint when no table name is passed.
+ """
+ database = mocker.MagicMock()
+ mocker.patch("superset.databases.api.DatabaseDAO.find_by_id", return_value=database)
+
+ response = client.get("/api/v1/database/1/table_metadata/extra/?schema=s&catalog=c")
+ assert response.status_code == 422
+ assert response.json == {
+ "errors": [
+ {
+ "message": "An error happened when validating the request",
+ "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR",
+ "level": "error",
+ "extra": {
+ "messages": {"table": ["Missing data for required field."]},
+ "issue_codes": [
+ {
+ "code": 1020,
+ "message": "Issue 1020 - The submitted payload has the incorrect schema.",
+ }
+ ],
+ },
+ }
+ ]
+ }
+
+
+def test_table_extra_metadata_slashes(
+ mocker: MockFixture,
+ client: Any,
+ full_api_access: None,
+) -> None:
+ """
+ Test the `table_extra_metadata` endpoint with names that have slashes.
+ """
+ database = mocker.MagicMock()
+ database.db_engine_spec.get_extra_table_metadata.return_value = {"hello": "world"}
+ mocker.patch("superset.databases.api.DatabaseDAO.find_by_id", return_value=database)
+ mocker.patch("superset.databases.api.security_manager.raise_for_access")
+
+ client.get("/api/v1/database/1/table_metadata/extra/?table=foo/bar")
+ database.db_engine_spec.get_extra_table_metadata.assert_called_with(
+ database,
+ Table("foo/bar"),
+ )
+
+
+def test_table_extra_metadata_invalid_database(
+ mocker: MockFixture,
+ client: Any,
+ full_api_access: None,
+) -> None:
+ """
+ Test the `table_extra_metadata` endpoint when the database is invalid.
+ """
+ mocker.patch("superset.databases.api.DatabaseDAO.find_by_id", return_value=None)
+
+ response = client.get("/api/v1/database/1/table_metadata/extra/?table=t")
+ assert response.status_code == 404
+ assert response.json == {
+ "errors": [
+ {
+ "message": "No such database",
+ "error_type": "DATABASE_NOT_FOUND_ERROR",
+ "level": "error",
+ "extra": {
+ "issue_codes": [
+ {
+ "code": 1011,
+ "message": "Issue 1011 - Superset encountered an unexpected error.",
+ },
+ {
+ "code": 1036,
+ "message": "Issue 1036 - The database was deleted.",
+ },
+ ]
+ },
+ }
+ ]
+ }
+
+
+def test_table_extra_metadata_unauthorized(
+ mocker: MockFixture,
+ client: Any,
+ full_api_access: None,
+) -> None:
+ """
+ Test the `table_extra_metadata` endpoint when the user is unauthorized.
+ """
+ database = mocker.MagicMock()
+ mocker.patch("superset.databases.api.DatabaseDAO.find_by_id", return_value=database)
+ mocker.patch(
+ "superset.databases.api.security_manager.raise_for_access",
+ side_effect=SupersetSecurityException(
+ SupersetError(
+ error_type=SupersetErrorType.TABLE_SECURITY_ACCESS_ERROR,
+ message="You don't have access to the table",
+ level=ErrorLevel.ERROR,
+ )
+ ),
+ )
+
+ response = client.get("/api/v1/database/1/table_metadata/extra/?table=t")
+ assert response.status_code == 404
+ assert response.json == {
+ "errors": [
+ {
+ "message": "No such table",
+ "error_type": "TABLE_NOT_FOUND_ERROR",
+ "level": "error",
+ "extra": None,
+ }
+ ]
+ }
diff --git a/tests/unit_tests/db_engine_specs/test_trino.py b/tests/unit_tests/db_engine_specs/test_trino.py
index 308902c901..7ce9199c5b 100644
--- a/tests/unit_tests/db_engine_specs/test_trino.py
+++ b/tests/unit_tests/db_engine_specs/test_trino.py
@@ -37,6 +37,7 @@ from superset.db_engine_specs.exceptions import (
SupersetDBAPIOperationalError,
SupersetDBAPIProgrammingError,
)
+from superset.sql_parse import Table
from superset.superset_typing import ResultSetColumnType, SQLAColumnType
from superset.utils.core import GenericDataType
from tests.unit_tests.db_engine_specs.utils import (
@@ -310,7 +311,7 @@ def test_convert_dttm(
assert_convert_dttm(TrinoEngineSpec, target_type, expected_result, dttm)
-def test_extra_table_metadata() -> None:
+def test_get_extra_table_metadata() -> None:
from superset.db_engine_specs.trino import TrinoEngineSpec
db_mock = Mock()
@@ -320,7 +321,10 @@ def test_extra_table_metadata() -> None:
db_mock.get_extra = Mock(return_value={})
db_mock.has_view_by_name = Mock(return_value=None)
db_mock.get_df = Mock(return_value=pd.DataFrame({"ds": ["01-01-19"], "hour": [1]}))
- result = TrinoEngineSpec.extra_table_metadata(db_mock, "test_table", "test_schema")
+ result = TrinoEngineSpec.get_extra_table_metadata(
+ db_mock,
+ Table("test_table", "test_schema"),
+ )
assert result["partitions"]["cols"] == ["ds", "hour"]
assert result["partitions"]["latest"] == {"ds": "01-01-19", "hour": 1}