iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat: expand new chart data endpoint coverage (#9903) 

* feat: implement new chart API for additional components

* Fix python tests

* Fix tests

* Fix lint

* fix camel case error in requestParams

* lint

* fix samples row limit

* Add samples row limit to config

* remove unnecessary code

* lint

* Address review comments
This commit is contained in:
Ville Brofeldt 2020-06-02 10:47:28 +03:00 committed by GitHub
parent c7618ee54b
commit 38a6bd79da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 196 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
superset-frontend/spec/javascripts/chart/chartActions_spec.js
View File

@ -55,7 +55,13 @@ describe('chart actions', () => {
.callsFake(() => ({ get: () => fakeMetadata }));
buildQueryRegistryStub = sinon
.stub(chartlib, 'getChartBuildQueryRegistry')
.callsFake(() => ({ get: () => () => ({ some_param: 'fake query!' }) }));
.callsFake(() => ({
get: () => () => ({
some_param: 'fake query!',
result_type: 'full',
result_format: 'json',
}),
}));
});
afterEach(() => {
@ -76,7 +82,11 @@ describe('chart actions', () => {
expect(fetchMock.calls(V1_URL)).toHaveLength(1);
expect(fetchMock.calls(V1_URL)[0][1].body).toBe(
JSON.stringify({ some_param: 'fake query!' }),
JSON.stringify({
some_param: 'fake query!',
result_type: 'full',
result_format: 'json',
}),
);
expect(dispatch.args[0][0].type).toBe(actions.CHART_UPDATE_STARTED);
});
@ -153,6 +163,7 @@ describe('chart actions', () => {
return actionThunk(dispatch).then(() => {
// chart update, trigger query, update form data, fail
expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
expect(dispatch.callCount).toBe(5);
expect(dispatch.args[4][0].type).toBe(actions.CHART_UPDATE_TIMEOUT);
setupDefaultFetchMock();

204
superset-frontend/src/chart/chartAction.js
View File

@ -20,11 +20,13 @@
/* eslint no-param-reassign: ["error", { "props": false }] */
import moment from 'moment';
import { t } from '@superset-ui/translation';
import { getChartBuildQueryRegistry } from '@superset-ui/chart';
import { SupersetClient } from '@superset-ui/connection';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import {
shouldUseLegacyApi,
getChartBuildQueryRegistry,
getChartMetadataRegistry,
} from '@superset-ui/chart';
import { isFeatureEnabled, FeatureFlag } from '../featureFlags';
import {
getExploreUrl,
getAnnotationJsonUrl,
postForm,
@ -33,6 +35,7 @@ import {
requiresQuery,
ANNOTATION_SOURCE_TYPES,
} from '../modules/AnnotationTypes';
import { addDangerToast } from '../messageToasts/actions';
import { logEvent } from '../logger/actions';
import { Logger, LOG_ACTIONS_LOAD_CHART } from '../logger/LogUtils';
@ -99,6 +102,118 @@ export function annotationQueryFailed(annotation, queryResponse, key) {
return { type: ANNOTATION_QUERY_FAILED, annotation, queryResponse, key };
}
const shouldUseLegacyApi = formData => {
const { useLegacyApi } = getChartMetadataRegistry().get(formData.viz_type);
return useLegacyApi || false;
};
const legacyChartDataRequest = async (
formData,
resultFormat,
resultType,
force,
method = 'POST',
requestParams = {},
) => {
const endpointType = ['base', 'csv', 'results', 'samples'].includes(
resultType,
)
? resultType
: resultFormat;
const url = getExploreUrl({
formData,
endpointType,
force,
allowDomainSharding,
method,
requestParams: requestParams.dashboard_id
? { dashboard_id: requestParams.dashboard_id }
: {},
});
const querySettings = {
...requestParams,
url,
postPayload: { form_data: formData },
};
const clientMethod =
'GET' && isFeatureEnabled(FeatureFlag.CLIENT_CACHE)
? SupersetClient.get
: SupersetClient.post;
return clientMethod(querySettings).then(({ json }) => {
// Make the legacy endpoint return a payload that corresponds to the
// V1 chart data endpoint response signature.
return {
result: [json],
};
});
};
const v1ChartDataRequest = async (
formData,
resultFormat,
resultType,
force,
requestParams,
) => {
const buildQuery = await getChartBuildQueryRegistry().get(formData.viz_type);
const payload = buildQuery({
...formData,
force,
});
// TODO: remove once these are added to superset-ui/query
payload.result_type = resultType;
payload.result_format = resultFormat;
const querySettings = {
...requestParams,
endpoint: '/api/v1/chart/data',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload),
};
return SupersetClient.post(querySettings).then(({ json }) => {
return json;
});
};
export async function getChartDataRequest(
formData,
resultFormat = 'json',
resultType = 'full',
force = false,
method = 'POST',
requestParams = {},
) {
let querySettings = {
...requestParams,
};
if (allowDomainSharding) {
querySettings = {
...querySettings,
mode: 'cors',
credentials: 'include',
};
}
if (shouldUseLegacyApi(formData)) {
return legacyChartDataRequest(
formData,
resultFormat,
resultType,
force,
method,
querySettings,
);
}
return v1ChartDataRequest(
formData,
resultFormat,
resultType,
force,
querySettings,
);
}
export function runAnnotationQuery(
annotation,
timeout = 60,
@ -204,48 +319,6 @@ export function addChart(chart, key) {
return { type: ADD_CHART, chart, key };
}
function legacyChartDataRequest(
formData,
force,
method,
dashboardId,
requestParams,
) {
const url = getExploreUrl({
formData,
endpointType: 'json',
force,
allowDomainSharding,
method,
requestParams: dashboardId ? { dashboard_id: dashboardId } : {},
});
const querySettings = {
...requestParams,
url,
postPayload: { form_data: formData },
};
const clientMethod =
method === 'GET' && isFeatureEnabled(FeatureFlag.CLIENT_CACHE)
? SupersetClient.get
: SupersetClient.post;
return clientMethod(querySettings);
}
async function v1ChartDataRequest(formData, force, requestParams) {
const buildQuery = await getChartBuildQueryRegistry().get(formData.viz_type);
const payload = buildQuery({ ...formData, force });
const querySettings = {
...requestParams,
endpoint: '/api/v1/chart/data',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload),
};
return SupersetClient.post(querySettings);
}
export function exploreJSON(
formData,
force = false,
@ -258,41 +331,30 @@ export function exploreJSON(
const logStart = Logger.getTimestamp();
const controller = new AbortController();
const useLegacyApi = shouldUseLegacyApi(formData);
let requestParams = {
const requestParams = {
signal: controller.signal,
timeout: timeout * 1000,
};
if (dashboardId) requestParams.dashboard_id = dashboardId;
if (allowDomainSharding) {
requestParams = {
...requestParams,
mode: 'cors',
credentials: 'include',
};
}
const queryPromiseRaw = useLegacyApi
? legacyChartDataRequest(
formData,
force,
method,
dashboardId,
requestParams,
)
: v1ChartDataRequest(formData, force, requestParams);
const chartDataRequest = getChartDataRequest(
formData,
'json',
'full',
force,
method,
requestParams,
);
dispatch(chartUpdateStarted(controller, formData, key));
const queryPromiseCaught = queryPromiseRaw
.then(({ json }) => {
const chartDataRequestCaught = chartDataRequest
.then(response => {
// new API returns an object with an array of restults
// problem: json holds a list of results, when before we were just getting one result.
// `queryResponse` state is used all over the place.
// problem: response holds a list of results, when before we were just getting one result.
// How to make the entire app compatible with multiple results?
// For now just use the first result.
const result = useLegacyApi ? json : json.result[0];
const result = response.result[0];
dispatch(
logEvent(LOG_ACTIONS_LOAD_CHART, {
slice_id: key,
@ -348,7 +410,7 @@ export function exploreJSON(
const annotationLayers = formData.annotation_layers || [];
return Promise.all([
queryPromiseCaught,
chartDataRequestCaught,
dispatch(triggerQuery(false, key)),
dispatch(updateQueryFormData(formData, key)),
...annotationLayers.map(x =>

30
superset-frontend/src/explore/components/DisplayQueryButton.jsx
View File

@ -41,7 +41,7 @@ import { SupersetClient } from '@superset-ui/connection';
import getClientErrorObject from '../../utils/getClientErrorObject';
import CopyToClipboard from './../../components/CopyToClipboard';
import { getExploreUrl } from '../exploreUtils';
import { getChartDataRequest } from '../../chart/chartAction';
import Loading from '../../components/Loading';
import ModalTrigger from './../../components/ModalTrigger';
@ -87,33 +87,29 @@ export class DisplayQueryButton extends React.PureComponent {
this.openPropertiesModal = this.openPropertiesModal.bind(this);
this.closePropertiesModal = this.closePropertiesModal.bind(this);
}
beforeOpen(endpointType) {
beforeOpen(resultType) {
this.setState({ isLoading: true });
const url = getExploreUrl({
formData: this.props.latestQueryFormData,
endpointType,
});
SupersetClient.post({
url,
postPayload: { form_data: this.props.latestQueryFormData },
})
.then(({ json }) => {
getChartDataRequest(this.props.latestQueryFormData, 'json', resultType)
.then(response => {
// Currently displaying of only first query is supported
const result = response.result[0];
this.setState({
language: json.language,
query: json.query,
data: json.data,
language: result.language,
query: result.query,
data: result.data,
isLoading: false,
error: null,
});
})
.catch(response =>
.catch(response => {
getClientErrorObject(response).then(({ error, statusText }) => {
this.setState({
error: error || statusText || t('Sorry, An error occurred'),
isLoading: false,
});
}),
);
});
});
}
changeFilterText(event) {
this.setState({ filterText: event.target.value });

2
superset-frontend/src/explore/components/ExploreActionButtons.jsx
View File

@ -48,7 +48,7 @@ export default function ExploreActionButtons({
'disabled disabledButton': !canDownload,
});
const doExportCSV = exportChart.bind(this, latestQueryFormData, 'csv');
const doExportChart = exportChart.bind(this, latestQueryFormData, 'json');
const doExportChart = exportChart.bind(this, latestQueryFormData, 'results');
return (
<div className="btn-group results" role="group">

13
superset-frontend/src/explore/exploreUtils.js
View File

@ -18,8 +18,8 @@
*/
/* eslint camelcase: 0 */
import URI from 'urijs';
import { getChartMetadataRegistry } from '@superset-ui/chart';
import { availableDomains } from '../utils/hostNamesConfig';
import { SupersetClient } from '@superset-ui/connection';
import { allowCrossDomain, availableDomains } from '../utils/hostNamesConfig';
import { safeStringify } from '../utils/safeStringify';
const MAX_URL_LENGTH = 8000;
@ -67,7 +67,9 @@ export function getAnnotationJsonUrl(slice_id, form_data, isNative) {
export function getURIDirectory(endpointType = 'base') {
// Building the directory part of the URI
if (
['json', 'csv', 'query', 'results', 'samples'].indexOf(endpointType) >= 0
['full', 'json', 'csv', 'query', 'results', 'samples'].includes(
endpointType,
)
) {
return '/superset/explore_json/';
}
@ -107,11 +109,6 @@ export function getExploreLongUrl(
return url;
}
export function shouldUseLegacyApi(formData) {
const { useLegacyApi } = getChartMetadataRegistry().get(formData.viz_type);
return useLegacyApi || false;
}
export function getExploreUrl({
formData,
endpointType = 'base',

2
superset/charts/schemas.py
View File

@ -713,7 +713,7 @@ class ChartDataQueryContextSchema(Schema):
)
result_type = fields.String(
description="Type of results to return",
validate=validate.OneOf(choices=("query", "results", "samples")),
validate=validate.OneOf(choices=("full", "query", "results", "samples")),
)
result_format = fields.String(
description="Format of result payload",

23
superset/common/query_context.py
View File

@ -16,6 +16,7 @@
# under the License.
import copy
import logging
import math
import pickle as pkl
from datetime import datetime, timedelta
from typing import Any, ClassVar, Dict, List, Optional, Union
@ -50,8 +51,8 @@ class QueryContext:
queries: List[QueryObject]
force: bool
custom_cache_timeout: Optional[int]
response_type: utils.ChartDataResponseType
response_format: utils.ChartDataResponseFormat
result_type: utils.ChartDataResultType
result_format: utils.ChartDataResultFormat
# TODO: Type datasource and query_object dictionary with TypedDict when it becomes
# a vanilla python type https://github.com/python/mypy/issues/5288
@ -61,8 +62,8 @@ class QueryContext:
queries: List[Dict[str, Any]],
force: bool = False,
custom_cache_timeout: Optional[int] = None,
response_format: Optional[utils.ChartDataResponseFormat] = None,
response_type: Optional[utils.ChartDataResponseType] = None,
result_type: Optional[utils.ChartDataResultType] = None,
result_format: Optional[utils.ChartDataResultFormat] = None,
) -> None:
self.datasource = ConnectorRegistry.get_datasource(
str(datasource["type"]), int(datasource["id"]), db.session
@ -70,8 +71,8 @@ class QueryContext:
self.queries = [QueryObject(**query_obj) for query_obj in queries]
self.force = force
self.custom_cache_timeout = custom_cache_timeout
self.response_format = response_format or utils.ChartDataResponseFormat.JSON
self.response_type = response_type or utils.ChartDataResponseType.RESULTS
self.result_type = result_type or utils.ChartDataResultType.FULL
self.result_format = result_format or utils.ChartDataResultFormat.JSON
def get_query_result(self, query_object: QueryObject) -> Dict[str, Any]:
"""Returns a pandas dataframe based on the query object"""
@ -134,7 +135,7 @@ class QueryContext:
def get_data(
self, df: pd.DataFrame,
) -> Union[str, List[Dict[str, Any]]]: # pylint: disable=no-self-use
if self.response_format == utils.ChartDataResponseFormat.CSV:
if self.result_format == utils.ChartDataResultFormat.CSV:
include_index = not isinstance(df.index, pd.RangeIndex)
result = df.to_csv(index=include_index, **config["CSV_EXPORT"])
return result or ""
@ -143,18 +144,18 @@ class QueryContext:
def get_single_payload(self, query_obj: QueryObject) -> Dict[str, Any]:
"""Returns a payload of metadata and data"""
if self.response_type == utils.ChartDataResponseType.QUERY:
if self.result_type == utils.ChartDataResultType.QUERY:
return {
"query": self.datasource.get_query_str(query_obj.to_dict()),
"language": self.datasource.query_language,
}
if self.response_type == utils.ChartDataResponseType.SAMPLES:
row_limit = query_obj.row_limit or 1000
if self.result_type == utils.ChartDataResultType.SAMPLES:
row_limit = query_obj.row_limit or math.inf
query_obj = copy.copy(query_obj)
query_obj.groupby = []
query_obj.metrics = []
query_obj.post_processing = []
query_obj.row_limit = row_limit
query_obj.row_limit = min(row_limit, config["SAMPLES_ROW_LIMIT"])
query_obj.columns = [o.column_name for o in self.datasource.columns]
payload = self.get_df_payload(query_obj)

2
superset/config.py
View File

@ -109,6 +109,8 @@ VERSION_SHA = _try_json_readsha(VERSION_INFO_FILE, VERSION_SHA_LENGTH)
ROW_LIMIT = 50000
VIZ_ROW_LIMIT = 10000
# max rows retreieved when requesting samples from datasource in explore view
SAMPLES_ROW_LIMIT = 1000
# max rows retrieved by filter select auto complete
FILTER_SELECT_ROW_LIMIT = 10000
SUPERSET_WORKERS = 2 # deprecated

5
superset/utils/core.py
View File

@ -1383,17 +1383,18 @@ class FilterOperator(str, Enum):
REGEX = "REGEX"
class ChartDataResponseType(str, Enum):
class ChartDataResultType(str, Enum):
"""
Chart data response type
"""
FULL = "full"
QUERY = "query"
RESULTS = "results"
SAMPLES = "samples"
class ChartDataResponseFormat(str, Enum):
class ChartDataResultFormat(str, Enum):
"""
Chart data response format
"""

14
superset/views/core.py
View File

@ -637,7 +637,7 @@ class Superset(BaseSupersetView):
return self.json_response({"data": viz_obj.get_samples()})
def generate_json(self, viz_obj, response_type: Optional[str] = None) -> Response:
if response_type == utils.ChartDataResponseFormat.CSV:
if response_type == utils.ChartDataResultFormat.CSV:
return CsvResponse(
viz_obj.get_csv(),
status=200,
@ -645,13 +645,13 @@ class Superset(BaseSupersetView):
mimetype="application/csv",
)
if response_type == utils.ChartDataResponseType.QUERY:
if response_type == utils.ChartDataResultType.QUERY:
return self.get_query_string_response(viz_obj)
if response_type == utils.ChartDataResponseType.RESULTS:
if response_type == utils.ChartDataResultType.RESULTS:
return self.get_raw_results(viz_obj)
if response_type == utils.ChartDataResponseType.SAMPLES:
if response_type == utils.ChartDataResultType.SAMPLES:
return self.get_samples(viz_obj)
payload = viz_obj.get_payload()
@ -728,9 +728,9 @@ class Superset(BaseSupersetView):
payloads based on the request args in the first block
TODO: break into one endpoint for each return shape"""
response_type = utils.ChartDataResponseFormat.JSON.value
responses = [resp_format for resp_format in utils.ChartDataResponseFormat]
responses.extend([resp_type for resp_type in utils.ChartDataResponseType])
response_type = utils.ChartDataResultFormat.JSON.value
responses = [resp_format for resp_format in utils.ChartDataResultFormat]
responses.extend([resp_type for resp_type in utils.ChartDataResultType])
for response_option in responses:
if request.args.get(response_option) == "true":
response_type = response_option

2
superset/viz.py
View File

@ -212,7 +212,7 @@ class BaseViz:
{
"groupby": [],
"metrics": [],
"row_limit": 1000,
"row_limit": config["SAMPLES_ROW_LIMIT"],
"columns": [o.column_name for o in self.datasource.columns],
}
)

10
tests/query_context_tests.py
View File

@ -20,8 +20,8 @@ from superset.charts.schemas import ChartDataQueryContextSchema
from superset.common.query_context import QueryContext
from superset.connectors.connector_registry import ConnectorRegistry
from superset.utils.core import (
ChartDataResponseFormat,
ChartDataResponseType,
ChartDataResultFormat,
ChartDataResultType,
TimeRangeEndpoint,
)
from tests.base_tests import SupersetTestCase
@ -144,7 +144,7 @@ class QueryContextTests(SupersetTestCase):
table_name = "birth_names"
table = self.get_table_by_name(table_name)
payload = get_query_context(table.name, table.id, table.type)
payload["response_format"] = ChartDataResponseFormat.CSV.value
payload["result_format"] = ChartDataResultFormat.CSV.value
payload["queries"][0]["row_limit"] = 10
query_context = QueryContext(**payload)
responses = query_context.get_payload()
@ -161,7 +161,7 @@ class QueryContextTests(SupersetTestCase):
table_name = "birth_names"
table = self.get_table_by_name(table_name)
payload = get_query_context(table.name, table.id, table.type)
payload["response_type"] = ChartDataResponseType.SAMPLES.value
payload["result_type"] = ChartDataResultType.SAMPLES.value
payload["queries"][0]["row_limit"] = 5
query_context = QueryContext(**payload)
responses = query_context.get_payload()
@ -179,7 +179,7 @@ class QueryContextTests(SupersetTestCase):
table_name = "birth_names"
table = self.get_table_by_name(table_name)
payload = get_query_context(table.name, table.id, table.type)
payload["response_type"] = ChartDataResponseType.QUERY.value
payload["result_type"] = ChartDataResultType.QUERY.value
query_context = QueryContext(**payload)
responses = query_context.get_payload()
self.assertEqual(len(responses), 1)