This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 38a6bd7 feat: expand new chart data endpoint coverage (#9903)
38a6bd7 is described below
commit 38a6bd79da2be8c13a89a5f67f77faeb55c4e08b
Author: Ville Brofeldt <[email protected]>
AuthorDate: Tue Jun 2 10:47:28 2020 +0300
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
---
.../spec/javascripts/chart/chartActions_spec.js | 15 +-
superset-frontend/src/chart/chartAction.js | 204 ++++++++++++++-------
.../src/explore/components/DisplayQueryButton.jsx | 30 ++-
.../explore/components/ExploreActionButtons.jsx | 2 +-
superset-frontend/src/explore/exploreUtils.js | 13 +-
superset/charts/schemas.py | 2 +-
superset/common/query_context.py | 23 +--
superset/config.py | 2 +
superset/utils/core.py | 5 +-
superset/views/core.py | 14 +-
superset/viz.py | 2 +-
tests/query_context_tests.py | 10 +-
12 files changed, 196 insertions(+), 126 deletions(-)
diff --git a/superset-frontend/spec/javascripts/chart/chartActions_spec.js
b/superset-frontend/spec/javascripts/chart/chartActions_spec.js
index 5c67509..15566dd 100644
--- a/superset-frontend/spec/javascripts/chart/chartActions_spec.js
+++ b/superset-frontend/spec/javascripts/chart/chartActions_spec.js
@@ -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();
diff --git a/superset-frontend/src/chart/chartAction.js
b/superset-frontend/src/chart/chartAction.js
index 3b46ae1..e022872 100644
--- a/superset-frontend/src/chart/chartAction.js
+++ b/superset-frontend/src/chart/chartAction.js
@@ -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 =>
diff --git a/superset-frontend/src/explore/components/DisplayQueryButton.jsx
b/superset-frontend/src/explore/components/DisplayQueryButton.jsx
index 32d5f76..5f57700 100644
--- a/superset-frontend/src/explore/components/DisplayQueryButton.jsx
+++ b/superset-frontend/src/explore/components/DisplayQueryButton.jsx
@@ -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 });
diff --git a/superset-frontend/src/explore/components/ExploreActionButtons.jsx
b/superset-frontend/src/explore/components/ExploreActionButtons.jsx
index c606108..e517d9f 100644
--- a/superset-frontend/src/explore/components/ExploreActionButtons.jsx
+++ b/superset-frontend/src/explore/components/ExploreActionButtons.jsx
@@ -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">
diff --git a/superset-frontend/src/explore/exploreUtils.js
b/superset-frontend/src/explore/exploreUtils.js
index 63706e8..4ea7ce5 100644
--- a/superset-frontend/src/explore/exploreUtils.js
+++ b/superset-frontend/src/explore/exploreUtils.js
@@ -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',
diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py
index 9d01901..609a868 100644
--- a/superset/charts/schemas.py
+++ b/superset/charts/schemas.py
@@ -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",
diff --git a/superset/common/query_context.py b/superset/common/query_context.py
index 96fc733..2085258 100644
--- a/superset/common/query_context.py
+++ b/superset/common/query_context.py
@@ -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)
diff --git a/superset/config.py b/superset/config.py
index e0f22f7..32ce4ae 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -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
diff --git a/superset/utils/core.py b/superset/utils/core.py
index b23136d..de758dc 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -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
"""
diff --git a/superset/views/core.py b/superset/views/core.py
index 60454d9..6c7e2b2 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -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
diff --git a/superset/viz.py b/superset/viz.py
index f20d8ed..bf3c110 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -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],
}
)
diff --git a/tests/query_context_tests.py b/tests/query_context_tests.py
index 7d6117a..310df01 100644
--- a/tests/query_context_tests.py
+++ b/tests/query_context_tests.py
@@ -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)
