Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(SIP-39): Async query support for charts #11499

Merged
merged 50 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
67f7d5f
Generate JWT in Flask app
robdiciuccio Oct 21, 2020
1e8c039
Refactor chart data API query logic, add JWT validation and async worker
robdiciuccio Oct 22, 2020
a50750b
Add redis stream implementation, refactoring
robdiciuccio Oct 27, 2020
64fbfae
Add chart data cache endpoint, refactor QueryContext caching
robdiciuccio Oct 30, 2020
219ce77
Merge branch master
robdiciuccio Nov 10, 2020
e377b31
Typing, linting, refactoring
robdiciuccio Nov 10, 2020
f7ac5b6
pytest fixes and openapi schema update
robdiciuccio Nov 11, 2020
3a16283
Merge branch master
robdiciuccio Nov 18, 2020
6805180
Enforce caching be configured for async query init
robdiciuccio Nov 19, 2020
d5eef4f
Async query processing for explore_json endpoint
robdiciuccio Nov 19, 2020
467c6bb
Add /api/v1/async_event endpoint
robdiciuccio Nov 20, 2020
f867cd5
Async frontend for dashboards [WIP]
robdiciuccio Nov 24, 2020
8111ea6
Chart async error message support, refactoring
robdiciuccio Nov 30, 2020
3b41f16
Abstract asyncEvent middleware
robdiciuccio Nov 30, 2020
49b6b52
Async chart loading for Explore
robdiciuccio Nov 30, 2020
0e5c09e
Merge branch master
robdiciuccio Nov 30, 2020
e2dc30e
Pylint fixes
robdiciuccio Nov 30, 2020
276c84e
asyncEvent middleware -> TypeScript, JS linting
robdiciuccio Dec 1, 2020
4566c01
Chart data API: enforce forced_cache, add tests
robdiciuccio Dec 2, 2020
9eba0c4
Add tests for explore_json endpoints
robdiciuccio Dec 3, 2020
d2e4529
Add test for chart data cache enpoint (no login)
robdiciuccio Dec 3, 2020
0bd7d67
Consolidate set_and_log_cache and add STORE_CACHE_KEYS_IN_METADATA_DB…
robdiciuccio Dec 3, 2020
4441459
Add tests for tasks/async_queries and address PR comments
robdiciuccio Dec 3, 2020
5896799
Bypass non-JSON result formats for async queries
robdiciuccio Dec 3, 2020
5999bf3
Add tests for redux middleware
robdiciuccio Dec 4, 2020
17215a3
Merge branch master
robdiciuccio Dec 4, 2020
27e4548
Remove debug statement
robdiciuccio Dec 4, 2020
119cae7
Skip force_cached if no queryObj
robdiciuccio Dec 4, 2020
9666408
SunburstViz: don't modify self.form_data
robdiciuccio Dec 4, 2020
d2ef464
Merge branch 'rd/async-queries-mvp' of github.com:preset-io/incubator…
robdiciuccio Dec 4, 2020
a8607c0
Merge branch master
robdiciuccio Dec 7, 2020
e40eb45
Fix failing annotation test
robdiciuccio Dec 7, 2020
f491789
Resolve merge/lint issues
robdiciuccio Dec 7, 2020
f01740e
Reduce polling delay
robdiciuccio Dec 8, 2020
9e02017
Merge branch 'master' into rd/async-queries-mvp
robdiciuccio Dec 8, 2020
838c526
Fix new getClientErrorObject reference
robdiciuccio Dec 8, 2020
f0de265
Fix flakey unit tests
robdiciuccio Dec 8, 2020
066504f
/api/v1/async_event: increment redis stream ID, add tests
robdiciuccio Dec 9, 2020
d6c8a1d
PR feedback: refactoring, configuration
robdiciuccio Dec 9, 2020
fc5753a
Merge branch master
robdiciuccio Dec 9, 2020
088a49c
Fixup: remove debugging
robdiciuccio Dec 9, 2020
c9b871e
Fix typescript errors due to redux upgrade
robdiciuccio Dec 10, 2020
89925a5
Update UPDATING.md
robdiciuccio Dec 10, 2020
0ad7234
Fix failing py tests
robdiciuccio Dec 10, 2020
c72b4c6
asyncEvent_spec.js -> asyncEvent_spec.ts
robdiciuccio Dec 10, 2020
1fb7489
Refactor flakey Python 3.7 mock assertions
robdiciuccio Dec 10, 2020
024da76
Fix another shared state issue in Py tests
robdiciuccio Dec 10, 2020
887754b
Use 'sub' claim in JWT for user_id
robdiciuccio Dec 11, 2020
601ec51
Refactor async middleware config
robdiciuccio Dec 11, 2020
df673cf
Fixup: restore FeatureFlag boolean type
robdiciuccio Dec 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,11 @@ cd superset-frontend
npm run test
```

To run a single test file:
```bash
npm run test -- path/to/file.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't actually have to specify the full path.

jest AsyncEvents

should also work.

```

### Integration Testing

We use [Cypress](https://www.cypress.io/) for integration tests. Tests can be run by `tox -e cypress`. To open Cypress and explore tests first setup and run test server:
Expand Down
2 changes: 1 addition & 1 deletion UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ This file documents any backwards-incompatible changes in Superset and
assists people when migrating to a new version.

## Next

- [11499](https://github.com/apache/incubator-superset/pull/11499): Breaking change: `STORE_CACHE_KEYS_IN_METADATA_DB` config flag added (default=`False`) to write `CacheKey` records to the metadata DB. `CacheKey` recording was enabled by default previously.
- [11920](https://github.com/apache/incubator-superset/pull/11920): Undos the DB migration from [11714](https://github.com/apache/incubator-superset/pull/11714) to prevent adding new columns to the logs table. Deploying a sha between these two PRs may result in locking your DB.
- [11704](https://github.com/apache/incubator-superset/pull/11704) Breaking change: Jinja templating for SQL queries has been updated, removing default modules such as `datetime` and `random` and enforcing static template values. To restore or extend functionality, use `JINJA_CONTEXT_ADDONS` and `CUSTOM_TEMPLATE_PROCESSORS` in `superset_config.py`.
- [11714](https://github.com/apache/incubator-superset/pull/11714): Logs
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ combine_as_imports = true
include_trailing_comma = true
line_length = 88
known_first_party = superset
known_third_party =alembic,apispec,backoff,bleach,cachelib,celery,click,colorama,contextlib2,croniter,cryptography,dateutil,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,freezegun,geohash,geopy,humanize,isodate,jinja2,markdown,markupsafe,marshmallow,msgpack,numpy,pandas,parameterized,parsedatetime,pathlib2,pgsanity,polyline,prison,pyarrow,pyhive,pytest,pytz,retry,selenium,setuptools,simplejson,slack,sqlalchemy,sqlalchemy_utils,sqlparse,werkzeug,wtforms,wtforms_json,yaml
known_third_party =alembic,apispec,backoff,bleach,cachelib,celery,click,colorama,contextlib2,croniter,cryptography,dateutil,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,freezegun,geohash,geopy,humanize,isodate,jinja2,jwt,markdown,markupsafe,marshmallow,msgpack,numpy,pandas,parameterized,parsedatetime,pathlib2,pgsanity,polyline,prison,pyarrow,pyhive,pytest,pytz,redis,retry,selenium,setuptools,simplejson,slack,sqlalchemy,sqlalchemy_utils,sqlparse,werkzeug,wtforms,wtforms_json,yaml
multi_line_output = 3
order_by_type = false

Expand Down
265 changes: 265 additions & 0 deletions superset-frontend/spec/javascripts/middleware/asyncEvent_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import fetchMock from 'fetch-mock';
import sinon from 'sinon';
import * as featureFlags from 'src/featureFlags';
import initAsyncEvents from 'src/middleware/asyncEvent';

jest.useFakeTimers();

describe('asyncEvent middleware', () => {
const next = sinon.spy();
const state = {
charts: {
123: {
id: 123,
status: 'loading',
asyncJobId: 'foo123',
},
345: {
id: 345,
status: 'loading',
asyncJobId: 'foo345',
},
},
};
const events = [
{
status: 'done',
result_url: '/api/v1/chart/data/cache-key-1',
job_id: 'foo123',
channel_id: '999',
errors: [],
},
{
status: 'done',
result_url: '/api/v1/chart/data/cache-key-2',
job_id: 'foo345',
channel_id: '999',
errors: [],
},
];
const mockStore = {
getState: () => state,
dispatch: sinon.stub(),
};
const action = {
type: 'GENERIC_ACTION',
};
const EVENTS_ENDPOINT = 'glob:*/api/v1/async_event/*';
const CACHED_DATA_ENDPOINT = 'glob:*/api/v1/chart/data/*';
const config = {
GLOBAL_ASYNC_QUERIES_TRANSPORT: 'polling',
GLOBAL_ASYNC_QUERIES_POLLING_DELAY: 500,
};
let featureEnabledStub: any;

function setup() {
const getPendingComponents = sinon.stub();
const successAction = sinon.spy();
const errorAction = sinon.spy();
const testCallback = sinon.stub();
const testCallbackPromise = sinon.stub();
testCallbackPromise.returns(
new Promise(resolve => {
testCallback.callsFake(resolve);
}),
);

return {
getPendingComponents,
successAction,
errorAction,
testCallback,
testCallbackPromise,
};
}

beforeEach(() => {
fetchMock.get(EVENTS_ENDPOINT, {
status: 200,
body: { result: [] },
});
fetchMock.get(CACHED_DATA_ENDPOINT, {
status: 200,
body: { result: { some: 'data' } },
});
featureEnabledStub = sinon.stub(featureFlags, 'isFeatureEnabled');
featureEnabledStub.withArgs('GLOBAL_ASYNC_QUERIES').returns(true);
});
afterEach(() => {
fetchMock.reset();
next.resetHistory();
featureEnabledStub.restore();
});
afterAll(fetchMock.reset);

it('should initialize and call next', () => {
const { getPendingComponents, successAction, errorAction } = setup();
getPendingComponents.returns([]);
const asyncEventMiddleware = initAsyncEvents({
config,
getPendingComponents,
successAction,
errorAction,
});
asyncEventMiddleware(mockStore)(next)(action);
expect(next.callCount).toBe(1);
});

it('should fetch events when there are pending components', () => {
const {
getPendingComponents,
successAction,
errorAction,
testCallback,
testCallbackPromise,
} = setup();
getPendingComponents.returns(Object.values(state.charts));
const asyncEventMiddleware = initAsyncEvents({
config,
getPendingComponents,
successAction,
errorAction,
processEventsCallback: testCallback,
});

asyncEventMiddleware(mockStore)(next)(action);

return testCallbackPromise().then(() => {
expect(fetchMock.calls(EVENTS_ENDPOINT)).toHaveLength(1);
});
});

it('should fetch cached when there are successful events', () => {
const {
getPendingComponents,
successAction,
errorAction,
testCallback,
testCallbackPromise,
} = setup();
fetchMock.reset();
fetchMock.get(EVENTS_ENDPOINT, {
status: 200,
body: { result: events },
});
fetchMock.get(CACHED_DATA_ENDPOINT, {
status: 200,
body: { result: { some: 'data' } },
});
getPendingComponents.returns(Object.values(state.charts));
const asyncEventMiddleware = initAsyncEvents({
config,
getPendingComponents,
successAction,
errorAction,
processEventsCallback: testCallback,
});

asyncEventMiddleware(mockStore)(next)(action);

return testCallbackPromise().then(() => {
expect(fetchMock.calls(EVENTS_ENDPOINT)).toHaveLength(1);
expect(fetchMock.calls(CACHED_DATA_ENDPOINT)).toHaveLength(2);
expect(successAction.callCount).toBe(2);
});
});

it('should call errorAction for cache fetch error responses', () => {
const {
getPendingComponents,
successAction,
errorAction,
testCallback,
testCallbackPromise,
} = setup();
fetchMock.reset();
fetchMock.get(EVENTS_ENDPOINT, {
status: 200,
body: { result: events },
});
fetchMock.get(CACHED_DATA_ENDPOINT, {
status: 400,
body: { errors: ['error'] },
});
getPendingComponents.returns(Object.values(state.charts));
const asyncEventMiddleware = initAsyncEvents({
config,
getPendingComponents,
successAction,
errorAction,
processEventsCallback: testCallback,
});

asyncEventMiddleware(mockStore)(next)(action);

return testCallbackPromise().then(() => {
expect(fetchMock.calls(EVENTS_ENDPOINT)).toHaveLength(1);
expect(fetchMock.calls(CACHED_DATA_ENDPOINT)).toHaveLength(2);
expect(errorAction.callCount).toBe(2);
});
});

it('should handle event fetching error responses', () => {
const {
getPendingComponents,
successAction,
errorAction,
testCallback,
testCallbackPromise,
} = setup();
fetchMock.reset();
fetchMock.get(EVENTS_ENDPOINT, {
status: 400,
body: { message: 'error' },
});
getPendingComponents.returns(Object.values(state.charts));
const asyncEventMiddleware = initAsyncEvents({
config,
getPendingComponents,
successAction,
errorAction,
processEventsCallback: testCallback,
});

asyncEventMiddleware(mockStore)(next)(action);

return testCallbackPromise().then(() => {
expect(fetchMock.calls(EVENTS_ENDPOINT)).toHaveLength(1);
});
});

it('should not fetch events when async queries are disabled', () => {
featureEnabledStub.restore();
featureEnabledStub = sinon.stub(featureFlags, 'isFeatureEnabled');
featureEnabledStub.withArgs('GLOBAL_ASYNC_QUERIES').returns(false);
const { getPendingComponents, successAction, errorAction } = setup();
getPendingComponents.returns(Object.values(state.charts));
const asyncEventMiddleware = initAsyncEvents({
config,
getPendingComponents,
successAction,
errorAction,
});

asyncEventMiddleware(mockStore)(next)(action);
expect(getPendingComponents.called).toBe(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import { ErrorTypeEnum } from 'src/components/ErrorMessage/types';
import getClientErrorObject from 'src/utils/getClientErrorObject';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';

describe('getClientErrorObject()', () => {
it('Returns a Promise', () => {
Expand Down
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 @@ -30,7 +30,7 @@ import {
addSuccessToast as addSuccessToastAction,
addWarningToast as addWarningToastAction,
} from '../../messageToasts/actions/index';
import getClientErrorObject from '../../utils/getClientErrorObject';
import { getClientErrorObject } from '../../utils/getClientErrorObject';
import COMMON_ERR_MESSAGES from '../../utils/errorMessages';

export const RESET_STATE = 'RESET_STATE';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import Button from 'src/components/Button';
import CopyToClipboard from '../../components/CopyToClipboard';
import { storeQuery } from '../../utils/common';
import getClientErrorObject from '../../utils/getClientErrorObject';
import { getClientErrorObject } from '../../utils/getClientErrorObject';
import withToasts from '../../messageToasts/enhancers/withToasts';

const propTypes = {
Expand Down
13 changes: 12 additions & 1 deletion superset-frontend/src/chart/chartAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
import { addDangerToast } from '../messageToasts/actions';
import { logEvent } from '../logger/actions';
import { Logger, LOG_ACTIONS_LOAD_CHART } from '../logger/LogUtils';
import getClientErrorObject from '../utils/getClientErrorObject';
import { getClientErrorObject } from '../utils/getClientErrorObject';
import { allowCrossDomain as domainShardingEnabled } from '../utils/hostNamesConfig';

export const CHART_UPDATE_STARTED = 'CHART_UPDATE_STARTED';
Expand Down Expand Up @@ -66,6 +66,11 @@ export function chartUpdateFailed(queryResponse, key) {
return { type: CHART_UPDATE_FAILED, queryResponse, key };
}

export const CHART_UPDATE_QUEUED = 'CHART_UPDATE_QUEUED';
export function chartUpdateQueued(asyncJobMeta, key) {
return { type: CHART_UPDATE_QUEUED, asyncJobMeta, key };
}

export const CHART_RENDERING_FAILED = 'CHART_RENDERING_FAILED';
export function chartRenderingFailed(error, key, stackTrace) {
return { type: CHART_RENDERING_FAILED, error, key, stackTrace };
Expand Down Expand Up @@ -356,6 +361,12 @@ export function exploreJSON(

const chartDataRequestCaught = chartDataRequest
.then(response => {
if (isFeatureEnabled(FeatureFlag.GLOBAL_ASYNC_QUERIES)) {
// deal with getChartDataRequest transforming the response data
const result = 'result' in response ? response.result[0] : response;
return dispatch(chartUpdateQueued(result, key));
}

// new API returns an object with an array of restults
// 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?
Expand Down
8 changes: 8 additions & 0 deletions superset-frontend/src/chart/chartReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ export default function chartReducer(charts = {}, action) {
chartUpdateEndTime: now(),
};
},
[actions.CHART_UPDATE_QUEUED](state) {
return {
...state,
asyncJobId: action.asyncJobMeta.job_id,
chartStatus: 'loading',
chartUpdateEndTime: now(),
};
},
[actions.CHART_RENDERING_SUCCEEDED](state) {
return { ...state, chartStatus: 'rendered', chartUpdateEndTime: now() };
},
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/components/AsyncSelect.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import PropTypes from 'prop-types';
// TODO: refactor this with `import { AsyncSelect } from src/components/Select`
import { Select } from 'src/components/Select';
import { t, SupersetClient } from '@superset-ui/core';
import getClientErrorObject from '../utils/getClientErrorObject';
import { getClientErrorObject } from '../utils/getClientErrorObject';

const propTypes = {
dataEndpoint: PropTypes.string.isRequired,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/actions/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
updateDirectPathToFilter,
} from './dashboardFilters';
import { applyDefaultFormData } from '../../explore/store';
import getClientErrorObject from '../../utils/getClientErrorObject';
import { getClientErrorObject } from '../../utils/getClientErrorObject';
import { SAVE_TYPE_OVERWRITE } from '../util/constants';
import {
addSuccessToast,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/actions/datasources.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import { SupersetClient } from '@superset-ui/core';
import getClientErrorObject from '../../utils/getClientErrorObject';
import { getClientErrorObject } from '../../utils/getClientErrorObject';

export const SET_DATASOURCE = 'SET_DATASOURCE';
export function setDatasource(datasource, key) {
Expand Down
Loading