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

Feature flag for client cache #7348

Merged
merged 9 commits into from
Apr 24, 2019
Merged
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions superset/assets/cypress/integration/dashboard/controls.js
Original file line number Diff line number Diff line change
@@ -38,8 +38,8 @@ export default () => describe('top-level controls', () => {
.forEach((slice) => {
const sliceRequest = `getJson_${slice.slice_id}`;
sliceRequests.push(`@${sliceRequest}`);
const formData = `{"slice_id":${slice.slice_id},"viz_type":"${slice.form_data.viz_type}"}`;
cy.route('GET', `/superset/explore_json/?form_data=${formData}`).as(sliceRequest);
const formData = `{"slice_id":${slice.slice_id}}`;
cy.route('POST', `/superset/explore_json/?form_data=${formData}`).as(sliceRequest);

const forceRefresh = `postJson_${slice.slice_id}_force`;
forceRefreshRequests.push(`@${forceRefresh}`);
4 changes: 2 additions & 2 deletions superset/assets/cypress/integration/dashboard/edit_mode.js
Original file line number Diff line number Diff line change
@@ -28,9 +28,9 @@ export default () => describe('edit mode', () => {
const bootstrapData = JSON.parse(data[0].dataset.bootstrap);
const dashboard = bootstrapData.dashboard_data;
const boxplotChartId = dashboard.slices.find(slice => (slice.form_data.viz_type === 'box_plot')).slice_id;
const formData = `{"slice_id":${boxplotChartId},"viz_type":"box_plot"}`;
const formData = `{"slice_id":${boxplotChartId}}`;
const boxplotRequest = `/superset/explore_json/?form_data=${formData}`;
cy.route('GET', boxplotRequest).as('boxplotRequest');
cy.route('POST', boxplotRequest).as('boxplotRequest');
});

cy.get('.dashboard-header').contains('Edit dashboard').click();
4 changes: 2 additions & 2 deletions superset/assets/cypress/integration/dashboard/filter.js
Original file line number Diff line number Diff line change
@@ -39,9 +39,9 @@ export default () => describe('dashboard filter', () => {
it('should apply filter', () => {
const aliases = [];

const formData = `{"slice_id":${filterId},"viz_type":"filter_box"}`;
const formData = `{"slice_id":${filterId}}`;
const filterRoute = `/superset/explore_json/?form_data=${formData}`;
cy.route('GET', filterRoute).as('fetchFilter');
cy.route('POST', filterRoute).as('fetchFilter');
cy.wait('@fetchFilter');
sliceIds
.filter(id => (parseInt(id, 10) !== filterId))
4 changes: 2 additions & 2 deletions superset/assets/cypress/integration/dashboard/load.js
Original file line number Diff line number Diff line change
@@ -34,8 +34,8 @@ export default () => describe('load', () => {
// then define routes and create alias for each requests
slices.forEach((slice) => {
const alias = `getJson_${slice.slice_id}`;
const formData = `{"slice_id":${slice.slice_id},"viz_type":"${slice.form_data.viz_type}"}`;
cy.route('GET', `/superset/explore_json/?form_data=${formData}`).as(alias);
const formData = `{"slice_id":${slice.slice_id}}`;
cy.route('POST', `/superset/explore_json/?form_data=${formData}`).as(alias);
aliases.push(`@${alias}`);
});
});
4 changes: 2 additions & 2 deletions superset/assets/cypress/integration/dashboard/save.js
Original file line number Diff line number Diff line change
@@ -56,9 +56,9 @@ export default () => describe('save', () => {
cy.wait('@copyRequest');

// should have box_plot chart
const formData = `{"slice_id":${boxplotChartId},"viz_type":"box_plot"}`;
const formData = `{"slice_id":${boxplotChartId}}`;
const boxplotRequest = `/superset/explore_json/?form_data=${formData}`;
cy.route('GET', boxplotRequest).as('boxplotRequest');
cy.route('POST', boxplotRequest).as('boxplotRequest');
cy.wait('@boxplotRequest');
cy.get('.grid-container .box_plot').should('be.exist');

3 changes: 2 additions & 1 deletion superset/assets/src/chart/Chart.jsx
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ import PropTypes from 'prop-types';
import React from 'react';
import { Alert } from 'react-bootstrap';

import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import { Logger, LOG_ACTIONS_RENDER_CHART_CONTAINER } from '../logger/LogUtils';
import Loading from '../components/Loading';
import RefreshChartOverlay from '../components/RefreshChartOverlay';
@@ -70,7 +71,7 @@ class Chart extends React.PureComponent {
}
componentDidMount() {
if (this.props.triggerQuery) {
if (this.props.chartId > 0) {
if (this.props.chartId > 0 && isFeatureEnabled(FeatureFlag.CLIENT_CACHE)) {
// Load saved chart with a GET request
this.props.actions.getSavedChart(
this.props.formData,
5 changes: 4 additions & 1 deletion superset/assets/src/chart/chartAction.js
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@
/* eslint no-param-reassign: ["error", { "props": false }] */
import { t } from '@superset-ui/translation';
import { SupersetClient } from '@superset-ui/connection';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import { getExploreUrlAndPayload, getAnnotationJsonUrl } from '../explore/exploreUtils';
import { requiresQuery, ANNOTATION_SOURCE_TYPES } from '../modules/AnnotationTypes';
import { addDangerToast } from '../messageToasts/actions';
@@ -194,7 +195,9 @@ export function exploreJSON(formData, force = false, timeout = 60, key, method)
};
}

const clientMethod = method === 'GET' ? SupersetClient.get : SupersetClient.post;
const clientMethod = method === 'GET' && isFeatureEnabled(FeatureFlag.CLIENT_CACHE)
? SupersetClient.get
: SupersetClient.post;
const queryPromise = clientMethod(querySettings)
.then(({ json }) => {
dispatch(logEvent(LOG_ACTIONS_LOAD_CHART, {
1 change: 1 addition & 0 deletions superset/assets/src/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@
export enum FeatureFlag {
SCOPED_FILTER = 'SCOPED_FILTER',
OMNIBAR = 'OMNIBAR',
CLIENT_CACHE = 'CLIENT_CACHE',

Choose a reason for hiding this comment

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

is CLIENT_CACHE feature enabled by default or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not. If it's not present in config.py it's treated as false.

Choose a reason for hiding this comment

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

thanks! then dashboard's integration tests are based on default behavior, POST requests. I think you probably need to revert tests changes from #7032 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, doing it now :)

}

export type FeatureFlagMap = {
5 changes: 4 additions & 1 deletion superset/config.py
Original file line number Diff line number Diff line change
@@ -200,7 +200,10 @@
# For example, DEFAULT_FEATURE_FLAGS = { 'FOO': True, 'BAR': False } here
# and FEATURE_FLAGS = { 'BAR': True, 'BAZ': True } in superset_config.py
# will result in combined feature flags of { 'FOO': True, 'BAR': True, 'BAZ': True }
DEFAULT_FEATURE_FLAGS = {}
DEFAULT_FEATURE_FLAGS = {
# Experimental feature introducing a client (browser) cache
'CLIENT_CACHE': False,
}

# A function that receives a dict of all feature flags
# (DEFAULT_FEATURE_FLAGS merged with FEATURE_FLAGS)