Skip to content

Commit

Permalink
feat: deprecate /superset/extra_table_metadata migrate to api v1 (apa…
Browse files Browse the repository at this point in the history
…che#19921)

* feat: deprecate /superset/extra_table_metadata migrate to api v1

* use can_read to table_extra_metadata

* troubleshoot sqlite

* fix test

* fix test

* fix test

* fix frontend test on sqllab
  • Loading branch information
dpgaspar authored and jasonvank committed May 4, 2022
1 parent 6f16048 commit 422d955
Show file tree
Hide file tree
Showing 13 changed files with 152 additions and 13 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ function getTableMetadata(table, query, dispatch) {
function getTableExtendedMetadata(table, query, dispatch) {
return SupersetClient.get({
endpoint: encodeURI(
`/superset/extra_table_metadata/${query.dbId}/` +
`/api/v1/database/${query.dbId}/table_extra/` +
`${encodeURIComponent(table.name)}/${encodeURIComponent(
table.schema,
)}/`,
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/SqlLab/actions/sqlLab.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,10 @@ describe('async actions', () => {
fetchMock.delete(updateTableSchemaEndpoint, {});
fetchMock.post(updateTableSchemaEndpoint, JSON.stringify({ id: 1 }));

const getTableMetadataEndpoint = 'glob:*/api/v1/database/*';
const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/';
fetchMock.get(getTableMetadataEndpoint, {});
const getExtraTableMetadataEndpoint =
'glob:*/superset/extra_table_metadata/*';
'glob:**/api/v1/database/*/table_extra/*/*/';
fetchMock.get(getExtraTableMetadataEndpoint, {});

let isFeatureEnabledMock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {

if (!isSqllabView && dbId && name && schema) {
SupersetClient.get({
endpoint: `/superset/extra_table_metadata/${dbId}/${name}/${schema}/`,
endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`,
})
.then(({ json }: { json: Record<string, any> }) => {
if (json && json.partitions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class AdhocFilterControl extends React.Component {

if (!isSqllabView && dbId && name && schema) {
SupersetClient.get({
endpoint: `/superset/extra_table_metadata/${dbId}/${name}/${schema}/`,
endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`,
})
.then(({ json }) => {
if (json && json.partitions) {
Expand Down
1 change: 1 addition & 0 deletions superset/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class RouteMethod: # pylint: disable=too-few-public-methods
"schemas": "read",
"select_star": "read",
"table_metadata": "read",
"table_extra_metadata": "read",
"test_connection": "read",
"validate_parameters": "read",
"favorite_status": "read",
Expand Down
68 changes: 67 additions & 1 deletion superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
get_export_ids_schema,
SchemasResponseSchema,
SelectStarResponseSchema,
TableExtraMetadataResponseSchema,
TableMetadataResponseSchema,
)
from superset.databases.utils import get_table_metadata
Expand All @@ -71,7 +72,7 @@
from superset.extensions import security_manager
from superset.models.core import Database
from superset.superset_typing import FlaskResponse
from superset.utils.core import error_msg_from_exception
from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item
from superset.views.base_api import (
BaseSupersetModelRestApi,
requires_form_data,
Expand All @@ -89,6 +90,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
RouteMethod.EXPORT,
RouteMethod.IMPORT,
"table_metadata",
"table_extra_metadata",
"select_star",
"schemas",
"test_connection",
Expand Down Expand Up @@ -197,6 +199,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
DatabaseRelatedObjectsResponse,
DatabaseTestConnectionSchema,
DatabaseValidateParametersSchema,
TableExtraMetadataResponseSchema,
TableMetadataResponseSchema,
SelectStarResponseSchema,
SchemasResponseSchema,
Expand Down Expand Up @@ -527,6 +530,69 @@ def table_metadata(
self.incr_stats("success", self.table_metadata.__name__)
return self.response(200, **table_info)

@expose("/<int:pk>/table_extra/<table_name>/<schema_name>/", methods=["GET"])
@protect()
@check_datasource_access
@safe
@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, database: Database, table_name: str, schema_name: str
) -> FlaskResponse:
"""Table schema info
---
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: path
schema:
type: string
name: table_name
description: Table name
- in: path
schema:
type: string
name: schema_name
description: Table schema
responses:
200:
description: Table extra metadata information
content:
application/json:
schema:
$ref: "#/components/schemas/TableExtraMetadataResponseSchema"
400:
$ref: '#/components/responses/400'
401:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
422:
$ref: '#/components/responses/422'
500:
$ref: '#/components/responses/500'
"""
self.incr_stats("init", self.table_metadata.__name__)

parsed_schema = parse_js_uri_path_item(schema_name, eval_undefined=True)
table_name = parse_js_uri_path_item(table_name) # type: ignore
payload = database.db_engine_spec.extra_table_metadata(
database, table_name, parsed_schema
)
return self.response(200, **payload)

@expose("/<int:pk>/select_star/<table_name>/", methods=["GET"])
@expose("/<int:pk>/select_star/<table_name>/<schema_name>/", methods=["GET"])
@protect()
Expand Down
6 changes: 6 additions & 0 deletions superset/databases/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,12 @@ class TableMetadataResponseSchema(Schema):
selectStar = fields.String(description="SQL select star")


class TableExtraMetadataResponseSchema(Schema):
metadata = fields.Dict()
partitions = fields.Dict()
clustering = fields.Dict()


class SelectStarResponseSchema(Schema):
result = fields.String(description="SQL select star")

Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ def extra_table_metadata( # pylint: disable=unused-argument
cls,
database: "Database",
table_name: str,
schema_name: str,
schema_name: Optional[str],
) -> Dict[str, Any]:
"""
Returns engine-specific table metadata
Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def normalize_indexes(cls, indexes: List[Dict[str, Any]]) -> List[Dict[str, Any]

@classmethod
def extra_table_metadata(
cls, database: "Database", table_name: str, schema_name: str
cls, database: "Database", table_name: str, schema_name: Optional[str]
) -> Dict[str, Any]:
indexes = database.get_indexes(table_name, schema_name)
if not indexes:
Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/gsheets.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def extra_table_metadata(
cls,
database: "Database",
table_name: str,
schema_name: str,
schema_name: Optional[str],
) -> Dict[str, Any]:
engine = cls.get_engine(database, schema=schema_name)
with closing(engine.raw_connection()) as conn:
Expand Down
4 changes: 2 additions & 2 deletions superset/db_engine_specs/presto.py
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,7 @@ def expand_data( # pylint: disable=too-many-locals

@classmethod
def extra_table_metadata(
cls, database: "Database", table_name: str, schema_name: str
cls, database: "Database", table_name: str, schema_name: Optional[str]
) -> Dict[str, Any]:
metadata = {}

Expand Down Expand Up @@ -931,7 +931,7 @@ def extra_table_metadata(

@classmethod
def get_create_view(
cls, database: "Database", schema: str, table: str
cls, database: "Database", schema: Optional[str], table: str
) -> Optional[str]:
"""
Return a CREATE VIEW statement, or `None` if not a view.
Expand Down
6 changes: 6 additions & 0 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2154,6 +2154,12 @@ def sqllab_viz(self) -> FlaskResponse: # pylint: disable=no-self-use
def extra_table_metadata( # pylint: disable=no-self-use
self, database_id: int, table_name: str, schema: str
) -> FlaskResponse:
logger.warning(
"%s.extra_table_metadata "
"This API endpoint is deprecated and will be removed in version 3.0.0",
self.__class__.__name__,
)

parsed_schema = utils.parse_js_uri_path_item(schema, eval_undefined=True)
table_name = utils.parse_js_uri_path_item(table_name) # type: ignore
mydb = db.session.query(Database).filter_by(id=database_id).one()
Expand Down
64 changes: 62 additions & 2 deletions tests/integration_tests/databases/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,30 @@ def test_get_invalid_table_table_metadata(self):
Database API: Test get invalid table from table metadata
"""
example_db = get_example_database()
uri = f"api/v1/database/{example_db.id}/wrong_table/null/"
uri = f"api/v1/database/{example_db.id}/table/wrong_table/null/"
self.login(username="admin")
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)
data = json.loads(rv.data.decode("utf-8"))
if example_db.backend == "sqlite":
self.assertEqual(rv.status_code, 200)
self.assertEqual(
data,
{
"columns": [],
"comment": None,
"foreignKeys": [],
"indexes": [],
"name": "wrong_table",
"primaryKey": {"constrained_columns": None, "name": None},
"selectStar": "SELECT\nFROM wrong_table\nLIMIT 100\nOFFSET 0",
},
)
elif example_db.backend == "mysql":
self.assertEqual(rv.status_code, 422)
self.assertEqual(data, {"message": "`wrong_table`"})
else:
self.assertEqual(rv.status_code, 422)
self.assertEqual(data, {"message": "wrong_table"})

def test_get_table_metadata_no_db_permission(self):
"""
Expand All @@ -790,6 +810,46 @@ def test_get_table_metadata_no_db_permission(self):
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_get_table_extra_metadata(self):
"""
Database API: Test get table extra metadata info
"""
example_db = get_example_database()
self.login(username="admin")
uri = f"api/v1/database/{example_db.id}/table_extra/birth_names/null/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 200)
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(response, {})

def test_get_invalid_database_table_extra_metadata(self):
"""
Database API: Test get invalid database from table extra metadata
"""
database_id = 1000
self.login(username="admin")
uri = f"api/v1/database/{database_id}/table_extra/some_table/some_schema/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)

uri = "api/v1/database/some_database/table_extra/some_table/some_schema/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)

def test_get_invalid_table_table_extra_metadata(self):
"""
Database API: Test get invalid table from table extra metadata
"""
example_db = get_example_database()
uri = f"api/v1/database/{example_db.id}/table_extra/wrong_table/null/"
self.login(username="admin")
rv = self.client.get(uri)
data = json.loads(rv.data.decode("utf-8"))

self.assertEqual(rv.status_code, 200)
self.assertEqual(data, {})

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_get_select_star(self):
"""
Expand Down

0 comments on commit 422d955

Please sign in to comment.