diff --git a/superset-frontend/spec/javascripts/chart/chartActions_spec.js b/superset-frontend/spec/javascripts/chart/chartActions_spec.js index 5c6750940..15566dd50 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 3b46ae149..e022872ae 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 32d5f7630..5f57700d9 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 c606108c3..e517d9fed 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 (
diff --git a/superset-frontend/src/explore/exploreUtils.js b/superset-frontend/src/explore/exploreUtils.js index 63706e851..4ea7ce561 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 9d01901da..609a868fe 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 96fc73338..2085258c0 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 e0f22f7f2..32ce4ae6d 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 b23136d17..de758dc2c 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 60454d99e..6c7e2b221 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 f20d8edb1..bf3c110d6 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 7d6117ab6..310df012d 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)