mirror of
https://github.com/apache/superset.git
synced 2026-06-25 09:29:21 +00:00
Compare commits
11 Commits
fix/105973
...
alert-repo
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
242a41b82e | ||
|
|
faca187b51 | ||
|
|
6a6a4d4f1a | ||
|
|
0c02e0a3fb | ||
|
|
4c9b5c54f1 | ||
|
|
09c7ba14df | ||
|
|
3ec4bd23c4 | ||
|
|
f6ce105450 | ||
|
|
7bb4e82a82 | ||
|
|
2d78a8733c | ||
|
|
3261d10270 |
@@ -160,7 +160,7 @@ When enabled, Superset rejects webhook configurations that use `http://` URLs.
|
||||
|
||||
#### Retry Behavior
|
||||
|
||||
Superset automatically retries webhook deliveries on `429 Too Many Requests` and `5xx` server errors using exponential backoff.
|
||||
Superset automatically retries webhook deliveries on `429 Too Many Requests` and `5xx` server errors using exponential backoff. Retries are bounded to roughly 120 seconds of cumulative wall-clock time (worst case ~210 seconds, because the bound is checked against the time elapsed before each attempt, so the final request can begin just under the limit and still run its full request timeout), after which the delivery is abandoned.
|
||||
|
||||
### Kubernetes-specific
|
||||
|
||||
|
||||
@@ -90,6 +90,13 @@ const buildQuery: BuildQuery<TableChartFormData> = (
|
||||
let { metrics, orderby = [], columns = [] } = baseQueryObject;
|
||||
const { extras = {} } = baseQueryObject;
|
||||
let postProcessing: PostProcessingRule[] = [];
|
||||
// Capture the percent-metric `contribution` rule so it can be reused for
|
||||
// the totals query below. The totals query must rename percent-metric
|
||||
// columns the same way (`metric` -> `%metric`) so the footer can look them
|
||||
// up; without it the totals row renders 0.000%. We deliberately reuse only
|
||||
// this rule and not the full `postProcessing` array, which may also contain
|
||||
// a time-comparison operator that must not run on the single totals row.
|
||||
let contributionPostProcessing: PostProcessingRule | undefined;
|
||||
const nonCustomNorInheritShifts = ensureIsArray(
|
||||
formData.time_compare,
|
||||
).filter((shift: string) => shift !== 'custom' && shift !== 'inherit');
|
||||
@@ -157,15 +164,14 @@ const buildQuery: BuildQuery<TableChartFormData> = (
|
||||
metrics.concat(percentMetrics),
|
||||
getMetricLabel,
|
||||
);
|
||||
postProcessing = [
|
||||
{
|
||||
operation: 'contribution',
|
||||
options: {
|
||||
columns: percentMetricLabels,
|
||||
rename_columns: percentMetricLabels.map(x => `%${x}`),
|
||||
},
|
||||
contributionPostProcessing = {
|
||||
operation: 'contribution',
|
||||
options: {
|
||||
columns: percentMetricLabels,
|
||||
rename_columns: percentMetricLabels.map(x => `%${x}`),
|
||||
},
|
||||
];
|
||||
};
|
||||
postProcessing = [contributionPostProcessing];
|
||||
}
|
||||
// Add the operator for the time comparison if some is selected
|
||||
if (!isEmpty(timeOffsets)) {
|
||||
@@ -658,7 +664,13 @@ const buildQuery: BuildQuery<TableChartFormData> = (
|
||||
extras: totalsExtras, // Use extras with AG Grid WHERE removed
|
||||
row_limit: 0,
|
||||
row_offset: 0,
|
||||
post_processing: [],
|
||||
// Reapply only the percent-metric contribution rule so the totals row
|
||||
// exposes `%metric` keys (value/value = 100% on the single aggregated
|
||||
// row). The time-comparison operator from the main query is omitted on
|
||||
// purpose; it must not run against the single-row totals query.
|
||||
post_processing: contributionPostProcessing
|
||||
? [contributionPostProcessing]
|
||||
: [],
|
||||
order_desc: undefined, // we don't need orderby stuff here,
|
||||
orderby: undefined, // because this query will be used for get total aggregation.
|
||||
});
|
||||
|
||||
@@ -44,8 +44,3 @@ export const FILTER_CONDITION_BODY_INDEX = {
|
||||
} as const;
|
||||
|
||||
export const ROW_NUMBER_COL_ID = '__row_number__';
|
||||
|
||||
// Non-enumerable key used to attach a row's basic (increase/decrease) color
|
||||
// formatter to the row data object so it travels with the row through AG Grid
|
||||
// client-side sorting (#105973).
|
||||
export const BASIC_COLOR_FORMATTERS_ROW_KEY = '__basicColorFormatters__';
|
||||
|
||||
@@ -24,7 +24,6 @@ import {
|
||||
import { CustomCellRendererProps } from '@superset-ui/core/components/ThemedAgGridReact';
|
||||
import { BasicColorFormatterType, InputColumn, ValueRange } from '../types';
|
||||
import { useIsDark } from '../utils/useTableTheme';
|
||||
import getRowBasicColorFormatter from '../utils/getRowBasicColorFormatter';
|
||||
|
||||
const StyledTotalCell = styled.div`
|
||||
${() => `
|
||||
@@ -164,13 +163,13 @@ export const NumericCellRenderer = (
|
||||
let arrow = '';
|
||||
let arrowColor = '';
|
||||
if (hasBasicColorFormatters && col?.metricName) {
|
||||
const rowFormatter = getRowBasicColorFormatter(
|
||||
node,
|
||||
node?.rowIndex,
|
||||
basicColorFormatters,
|
||||
)?.[col.metricName];
|
||||
arrow = rowFormatter?.mainArrow;
|
||||
arrowColor = rowFormatter?.arrowColor?.toLowerCase();
|
||||
arrow =
|
||||
basicColorFormatters?.[node?.rowIndex as number]?.[col.metricName]
|
||||
?.mainArrow;
|
||||
arrowColor =
|
||||
basicColorFormatters?.[node?.rowIndex as number]?.[
|
||||
col.metricName
|
||||
]?.arrowColor?.toLowerCase();
|
||||
}
|
||||
|
||||
const alignment =
|
||||
|
||||
@@ -46,7 +46,6 @@ import {
|
||||
} from '@superset-ui/chart-controls';
|
||||
import isEqualColumns from './utils/isEqualColumns';
|
||||
import DateWithFormatter from './utils/DateWithFormatter';
|
||||
import { BASIC_COLOR_FORMATTERS_ROW_KEY } from './consts';
|
||||
import {
|
||||
DataColumnMeta,
|
||||
TableChartProps,
|
||||
@@ -704,23 +703,6 @@ const transformProps = (
|
||||
|
||||
const basicColorFormatters =
|
||||
comparisonColorEnabled && getBasicColorFormatter(baseQuery?.data, columns);
|
||||
|
||||
// Attach each row's basic (increase/decrease) color formatter to the row data
|
||||
// object so it travels with the row through AG Grid client-side sorting.
|
||||
// basicColorFormatters is built in the original query order and was previously
|
||||
// read positionally by the displayed rowIndex, which applied colors to the
|
||||
// wrong rows once the table was sorted (#105973). The property is
|
||||
// non-enumerable so it never leaks into exports, cross-filters or spreads.
|
||||
if (basicColorFormatters) {
|
||||
passedData.forEach((row, index) => {
|
||||
Object.defineProperty(row, BASIC_COLOR_FORMATTERS_ROW_KEY, {
|
||||
value: basicColorFormatters[index],
|
||||
enumerable: false,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
});
|
||||
}
|
||||
const columnColorFormatters =
|
||||
getColorFormatters(conditionalFormatting, passedData, theme) ?? [];
|
||||
|
||||
|
||||
@@ -24,7 +24,6 @@ import {
|
||||
} from '@superset-ui/chart-controls';
|
||||
import { CellClassParams } from '@superset-ui/core/components/ThemedAgGridReact';
|
||||
import { BasicColorFormatterType, InputColumn } from '../types';
|
||||
import getRowBasicColorFormatter from './getRowBasicColorFormatter';
|
||||
|
||||
type CellStyleParams = CellClassParams & {
|
||||
hasColumnColorFormatters: boolean | undefined;
|
||||
@@ -85,11 +84,8 @@ const getCellStyle = (params: CellStyleParams) => {
|
||||
col?.metricName &&
|
||||
node?.rowPinned !== 'bottom'
|
||||
) {
|
||||
backgroundColor = getRowBasicColorFormatter(
|
||||
node,
|
||||
rowIndex,
|
||||
basicColorFormatters,
|
||||
)?.[col.metricName]?.backgroundColor;
|
||||
backgroundColor =
|
||||
basicColorFormatters?.[rowIndex]?.[col.metricName]?.backgroundColor;
|
||||
}
|
||||
|
||||
const textAlign =
|
||||
|
||||
@@ -1,51 +0,0 @@
|
||||
/**
|
||||
* 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 { BASIC_COLOR_FORMATTERS_ROW_KEY } from '../consts';
|
||||
import { BasicColorFormatterType } from '../types';
|
||||
|
||||
type RowFormatters = { [key: string]: BasicColorFormatterType };
|
||||
|
||||
/**
|
||||
* Resolves the basic (increase/decrease) color formatters for a given AG Grid
|
||||
* row node.
|
||||
*
|
||||
* The formatter is attached to the row data object itself (see transformProps),
|
||||
* so it follows the row through client-side sorting. Looking it up positionally
|
||||
* by the displayed `rowIndex` was wrong once the user sorted the table, because
|
||||
* the displayed index no longer matched the original data order (#105973).
|
||||
*
|
||||
* Falls back to the positional array for safety when no attached formatter is
|
||||
* present.
|
||||
*/
|
||||
export default function getRowBasicColorFormatter(
|
||||
node: { data?: Record<string, unknown> } | undefined,
|
||||
rowIndex: number | null | undefined,
|
||||
basicColorFormatters: RowFormatters[] | undefined,
|
||||
): RowFormatters | undefined {
|
||||
const attached = node?.data?.[BASIC_COLOR_FORMATTERS_ROW_KEY] as
|
||||
| RowFormatters
|
||||
| undefined;
|
||||
if (attached) {
|
||||
return attached;
|
||||
}
|
||||
if (rowIndex == null) {
|
||||
return undefined;
|
||||
}
|
||||
return basicColorFormatters?.[rowIndex];
|
||||
}
|
||||
@@ -852,6 +852,75 @@ describe('plugin-chart-ag-grid-table', () => {
|
||||
expect(totalsQuery.columns).toEqual([]);
|
||||
expect(totalsQuery.row_limit).toBe(0);
|
||||
});
|
||||
|
||||
test('should reapply percent-metric contribution op to totals query', () => {
|
||||
// Regression test for #37627: when a percent metric is configured and
|
||||
// Show Summary (show_totals) is enabled, the totals query must rename
|
||||
// percent-metric columns (`metric` -> `%metric`) so the footer can
|
||||
// look them up. Otherwise the totals row renders 0.000%.
|
||||
const { queries } = buildQuery({
|
||||
...basicFormData,
|
||||
metrics: ['count'],
|
||||
percent_metrics: ['count'],
|
||||
show_totals: true,
|
||||
query_mode: QueryMode.Aggregate,
|
||||
});
|
||||
|
||||
// No server pagination -> queries[1] is the totals query.
|
||||
const totalsQuery = queries[1];
|
||||
const contributionRule = {
|
||||
operation: 'contribution',
|
||||
options: {
|
||||
columns: ['count'],
|
||||
rename_columns: ['%count'],
|
||||
},
|
||||
};
|
||||
|
||||
expect(queries[0].post_processing).toContainEqual(contributionRule);
|
||||
expect(totalsQuery.post_processing).toEqual([contributionRule]);
|
||||
});
|
||||
|
||||
test('should omit time-comparison op from totals post_processing', () => {
|
||||
// The totals query must reuse ONLY the contribution rule; the
|
||||
// time-comparison operator from the main query must not run against
|
||||
// the single-row totals query.
|
||||
const { queries } = buildQuery({
|
||||
...basicFormData,
|
||||
metrics: ['count'],
|
||||
percent_metrics: ['count'],
|
||||
show_totals: true,
|
||||
query_mode: QueryMode.Aggregate,
|
||||
time_compare: ['1 year ago'],
|
||||
comparison_type: 'values',
|
||||
});
|
||||
|
||||
const totalsQuery = queries[1];
|
||||
|
||||
// Exactly one op (contribution) — the time-comparison operator from the
|
||||
// main query must not be carried over to the single-row totals query.
|
||||
expect(totalsQuery.post_processing).toHaveLength(1);
|
||||
expect(totalsQuery.post_processing?.[0]).toMatchObject({
|
||||
operation: 'contribution',
|
||||
});
|
||||
// The reused rule matches the main query's contribution rule verbatim.
|
||||
expect(totalsQuery.post_processing?.[0]).toEqual(
|
||||
queries[0].post_processing?.find(
|
||||
op => op?.operation === 'contribution',
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('should leave totals post_processing empty without percent metrics', () => {
|
||||
const { queries } = buildQuery({
|
||||
...basicFormData,
|
||||
metrics: ['count'],
|
||||
show_totals: true,
|
||||
query_mode: QueryMode.Aggregate,
|
||||
});
|
||||
|
||||
const totalsQuery = queries[1];
|
||||
expect(totalsQuery.post_processing).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Integration - all filter types together', () => {
|
||||
|
||||
@@ -1,65 +0,0 @@
|
||||
/**
|
||||
* 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 getRowBasicColorFormatter from '../../src/utils/getRowBasicColorFormatter';
|
||||
import { BASIC_COLOR_FORMATTERS_ROW_KEY } from '../../src/consts';
|
||||
|
||||
const red = { sales: { backgroundColor: 'red', mainArrow: '↓', arrowColor: 'red' } };
|
||||
const green = {
|
||||
sales: { backgroundColor: 'green', mainArrow: '↑', arrowColor: 'green' },
|
||||
};
|
||||
|
||||
// Positional array in the original (unsorted) query order: row 0 -> green, row 1 -> red.
|
||||
const positional = [green, red] as any;
|
||||
|
||||
test('uses the formatter attached to the row, not the displayed rowIndex (#105973)', () => {
|
||||
// After sorting, the row whose original formatter is `red` is displayed first
|
||||
// (rowIndex 0). The positional lookup would wrongly return `green`.
|
||||
const rowData: Record<string, unknown> = { sales: 5 };
|
||||
Object.defineProperty(rowData, BASIC_COLOR_FORMATTERS_ROW_KEY, {
|
||||
value: red,
|
||||
enumerable: false,
|
||||
});
|
||||
const node = { data: rowData };
|
||||
|
||||
expect(getRowBasicColorFormatter(node, 0, positional)).toBe(red);
|
||||
expect(
|
||||
getRowBasicColorFormatter(node, 0, positional)?.sales.backgroundColor,
|
||||
).toBe('red');
|
||||
});
|
||||
|
||||
test('falls back to positional lookup when no formatter is attached', () => {
|
||||
const node = { data: { sales: 5 } };
|
||||
expect(getRowBasicColorFormatter(node, 1, positional)).toBe(red);
|
||||
});
|
||||
|
||||
test('returns undefined when nothing matches', () => {
|
||||
expect(getRowBasicColorFormatter(undefined, null, positional)).toBeUndefined();
|
||||
expect(
|
||||
getRowBasicColorFormatter({ data: {} }, null, positional),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
test('attached formatter is non-enumerable so it does not leak into the row', () => {
|
||||
const rowData: Record<string, unknown> = { sales: 5 };
|
||||
Object.defineProperty(rowData, BASIC_COLOR_FORMATTERS_ROW_KEY, {
|
||||
value: green,
|
||||
enumerable: false,
|
||||
});
|
||||
expect(Object.keys(rowData)).toEqual(['sales']);
|
||||
});
|
||||
@@ -86,6 +86,13 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
|
||||
let { metrics, orderby = [], columns = [] } = baseQueryObject;
|
||||
const { extras = {} } = baseQueryObject;
|
||||
const postProcessing: PostProcessingRule[] = [];
|
||||
// Capture the percent-metric `contribution` rule so it can be reused for
|
||||
// the totals query below. Without it the totals row's percent-metric
|
||||
// columns are keyed `metric` instead of `%metric`, so the footer renders
|
||||
// 0.000%. We reuse only this rule and not the full `postProcessing` array,
|
||||
// which may also contain a time-comparison operator that must not run on
|
||||
// the single totals row.
|
||||
let contributionPostProcessing: PostProcessingRule | undefined;
|
||||
const nonCustomNorInheritShifts = ensureIsArray(
|
||||
formData.time_compare,
|
||||
).filter((shift: string) => shift !== 'custom' && shift !== 'inherit');
|
||||
@@ -137,12 +144,6 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
|
||||
orderby = [[metrics[0], false]];
|
||||
}
|
||||
// add postprocessing for percent metrics only when in aggregation mode
|
||||
type PercentMetricCalculationMode = 'row_limit' | 'all_records';
|
||||
|
||||
const calculationMode: PercentMetricCalculationMode =
|
||||
(formData.percent_metric_calculation as PercentMetricCalculationMode) ||
|
||||
'row_limit';
|
||||
|
||||
if (percentMetrics && percentMetrics.length > 0) {
|
||||
const percentMetricsLabelsWithTimeComparison = isTimeComparison(
|
||||
formData,
|
||||
@@ -162,23 +163,14 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
|
||||
getMetricLabel,
|
||||
);
|
||||
|
||||
if (calculationMode === 'all_records') {
|
||||
postProcessing.push({
|
||||
operation: 'contribution',
|
||||
options: {
|
||||
columns: percentMetricLabels,
|
||||
rename_columns: percentMetricLabels.map(m => `%${m}`),
|
||||
},
|
||||
});
|
||||
} else {
|
||||
postProcessing.push({
|
||||
operation: 'contribution',
|
||||
options: {
|
||||
columns: percentMetricLabels,
|
||||
rename_columns: percentMetricLabels.map(m => `%${m}`),
|
||||
},
|
||||
});
|
||||
}
|
||||
contributionPostProcessing = {
|
||||
operation: 'contribution',
|
||||
options: {
|
||||
columns: percentMetricLabels,
|
||||
rename_columns: percentMetricLabels.map(m => `%${m}`),
|
||||
},
|
||||
};
|
||||
postProcessing.push(contributionPostProcessing);
|
||||
}
|
||||
|
||||
// Add the operator for the time comparison if some is selected
|
||||
@@ -357,7 +349,13 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
|
||||
columns: [],
|
||||
row_limit: 0,
|
||||
row_offset: 0,
|
||||
post_processing: [],
|
||||
// Reapply only the percent-metric contribution rule so the totals row
|
||||
// exposes `%metric` keys (value/value = 100% on the single aggregated
|
||||
// row). The time-comparison operator from the main query is omitted on
|
||||
// purpose; it must not run against the single-row totals query.
|
||||
post_processing: contributionPostProcessing
|
||||
? [contributionPostProcessing]
|
||||
: [],
|
||||
order_desc: undefined,
|
||||
orderby: undefined,
|
||||
});
|
||||
|
||||
@@ -236,6 +236,83 @@ describe('plugin-chart-table', () => {
|
||||
expect(queries).toHaveLength(1);
|
||||
expect(queries[0].post_processing).toEqual([]);
|
||||
});
|
||||
|
||||
test('should reapply contribution op to totals query in row_limit mode', () => {
|
||||
// Regression test for #37627: with a percent metric and Show Summary
|
||||
// (show_totals) enabled, the totals query must rename percent-metric
|
||||
// columns (`metric` -> `%metric`) so the footer can look them up.
|
||||
// Otherwise the totals row renders 0.000%.
|
||||
const formData = {
|
||||
...baseFormDataWithPercents,
|
||||
show_totals: true,
|
||||
};
|
||||
|
||||
const { queries } = buildQuery(formData);
|
||||
|
||||
// row_limit mode + show_totals -> [main, totals].
|
||||
expect(queries).toHaveLength(2);
|
||||
|
||||
const contributionRule = {
|
||||
operation: 'contribution',
|
||||
options: {
|
||||
columns: ['sum_sales'],
|
||||
rename_columns: ['%sum_sales'],
|
||||
},
|
||||
};
|
||||
|
||||
expect(queries[1]).toMatchObject({
|
||||
columns: [],
|
||||
post_processing: [contributionRule],
|
||||
});
|
||||
});
|
||||
|
||||
test('should omit time-comparison op from totals post_processing', () => {
|
||||
// The totals query must reuse ONLY the contribution rule; the
|
||||
// time-comparison operator from the main query must not run against
|
||||
// the single-row totals query.
|
||||
const formData = {
|
||||
...baseFormDataWithPercents,
|
||||
show_totals: true,
|
||||
time_compare: ['1 year ago'],
|
||||
comparison_type: 'values',
|
||||
};
|
||||
|
||||
const { queries } = buildQuery(formData);
|
||||
|
||||
// row_limit mode + show_totals -> [main, totals].
|
||||
expect(queries).toHaveLength(2);
|
||||
|
||||
const totalsQuery = queries[1];
|
||||
|
||||
// Exactly one op (contribution) — the time-comparison operator from the
|
||||
// main query must not be carried over to the single-row totals query.
|
||||
expect(totalsQuery.post_processing).toHaveLength(1);
|
||||
expect(totalsQuery.post_processing?.[0]).toMatchObject({
|
||||
operation: 'contribution',
|
||||
});
|
||||
// The reused rule matches the main query's contribution rule verbatim.
|
||||
expect(totalsQuery.post_processing?.[0]).toEqual(
|
||||
queries[0].post_processing?.find(
|
||||
op => op?.operation === 'contribution',
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('should leave totals post_processing empty without percent metrics', () => {
|
||||
const formData = {
|
||||
...basicFormData,
|
||||
query_mode: QueryMode.Aggregate,
|
||||
metrics: ['count'],
|
||||
percent_metrics: [],
|
||||
groupby: ['category'],
|
||||
show_totals: true,
|
||||
};
|
||||
|
||||
const { queries } = buildQuery(formData);
|
||||
|
||||
expect(queries).toHaveLength(2);
|
||||
expect(queries[1].post_processing).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Testing for server pagination with search filter', () => {
|
||||
|
||||
@@ -632,6 +632,35 @@ function processFile(filepath) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Application source trees that must be authored in TypeScript. Matches the
|
||||
* top-level `src/` directory as well as each package/plugin `src/` directory.
|
||||
*/
|
||||
const TS_ONLY_SOURCE_PATTERN =
|
||||
/^(src|packages\/[^/]+\/src|plugins\/[^/]+\/src)\//;
|
||||
|
||||
/**
|
||||
* Enforce the TypeScript-only frontend convention: no `.js`/`.jsx` files may be
|
||||
* added under the application source trees (including test files). Build
|
||||
* artifacts and root-level config files (e.g. `.storybook/preview.jsx`,
|
||||
* `webpack.config.js`) live outside these trees and are intentionally allowed.
|
||||
*
|
||||
* @param {string[]} candidateFiles paths relative to `superset-frontend/`
|
||||
*/
|
||||
function checkTypeScriptOnlySource(candidateFiles) {
|
||||
candidateFiles.forEach(file => {
|
||||
if (TS_ONLY_SOURCE_PATTERN.test(file) && /\.(js|jsx)$/.test(file)) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(
|
||||
`${RED}✗${RESET} ${file}: frontend source must be TypeScript. ` +
|
||||
`Rename to .ts/.tsx (the codebase is mid-migration to full ` +
|
||||
`TypeScript; no new .js/.jsx files in src/).`,
|
||||
);
|
||||
errorCount += 1;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Main function
|
||||
*/
|
||||
@@ -666,6 +695,22 @@ function main() {
|
||||
/packages\/superset-ui-core\/src\/color\/index\.ts/, // Core brand color constants
|
||||
];
|
||||
|
||||
// Enforce TypeScript-only source. Run this on the raw file list (before the
|
||||
// ignore patterns below strip out tests/stories) so that e.g. a new
|
||||
// `*.test.jsx` is still rejected.
|
||||
const tsOnlyCandidates =
|
||||
args.length === 0
|
||||
? glob.sync('{src,packages/*/src,plugins/*/src}/**/*.{js,jsx}', {
|
||||
ignore: [
|
||||
'**/node_modules/**',
|
||||
'**/esm/**',
|
||||
'**/lib/**',
|
||||
'**/dist/**',
|
||||
],
|
||||
})
|
||||
: args.map(f => f.replace(/^superset-frontend\//, ''));
|
||||
checkTypeScriptOnlySource(tsOnlyCandidates);
|
||||
|
||||
// If no files specified, check all
|
||||
if (files.length === 0) {
|
||||
files = glob.sync('src/**/*.{ts,tsx,js,jsx}', {
|
||||
@@ -706,22 +751,23 @@ function main() {
|
||||
if (files.length === 0) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.log('No files to check.');
|
||||
return;
|
||||
} else {
|
||||
// eslint-disable-next-line no-console
|
||||
console.log(
|
||||
`Checking ${files.length} files for Superset custom rules...\n`,
|
||||
);
|
||||
|
||||
files.forEach(file => {
|
||||
// Resolve the file path
|
||||
const resolvedPath = path.resolve(file);
|
||||
if (fs.existsSync(resolvedPath)) {
|
||||
processFile(resolvedPath);
|
||||
} else if (fs.existsSync(file)) {
|
||||
processFile(file);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// eslint-disable-next-line no-console
|
||||
console.log(`Checking ${files.length} files for Superset custom rules...\n`);
|
||||
|
||||
files.forEach(file => {
|
||||
// Resolve the file path
|
||||
const resolvedPath = path.resolve(file);
|
||||
if (fs.existsSync(resolvedPath)) {
|
||||
processFile(resolvedPath);
|
||||
} else if (fs.existsSync(file)) {
|
||||
processFile(file);
|
||||
}
|
||||
});
|
||||
|
||||
// eslint-disable-next-line no-console
|
||||
console.log(`\n${errorCount} errors, ${warningCount} warnings`);
|
||||
|
||||
@@ -740,4 +786,5 @@ module.exports = {
|
||||
checkNoFaIcons,
|
||||
checkI18nTemplates,
|
||||
checkUntranslatedStrings,
|
||||
checkTypeScriptOnlySource,
|
||||
};
|
||||
|
||||
@@ -42,6 +42,7 @@ import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
|
||||
import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers';
|
||||
import { URL_PARAMS } from 'src/constants';
|
||||
import { getUrlParam } from 'src/utils/urlUtils';
|
||||
import { sanitizeDocumentTitle } from 'src/utils/sanitizeDocumentTitle';
|
||||
import { setDatasetsStatus } from 'src/dashboard/actions/dashboardState';
|
||||
import { DASHBOARD_HEADER_ID } from 'src/dashboard/util/constants';
|
||||
import {
|
||||
@@ -337,7 +338,7 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
|
||||
// Update document title when dashboard title changes
|
||||
useEffect(() => {
|
||||
if (pageTitle) {
|
||||
document.title = pageTitle;
|
||||
document.title = sanitizeDocumentTitle(pageTitle);
|
||||
}
|
||||
}, [pageTitle]);
|
||||
|
||||
|
||||
@@ -66,6 +66,7 @@ import {
|
||||
LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS,
|
||||
} from 'src/logger/LogUtils';
|
||||
import { getUrlParam } from 'src/utils/urlUtils';
|
||||
import { sanitizeDocumentTitle } from 'src/utils/sanitizeDocumentTitle';
|
||||
import cx from 'classnames';
|
||||
import * as chartActions from 'src/components/Chart/chartAction';
|
||||
import { fetchDatasourceMetadata } from 'src/dashboard/actions/datasources';
|
||||
@@ -397,7 +398,7 @@ function ExploreViewContainer(props: ExploreViewContainerProps) {
|
||||
// Update document title when slice name changes
|
||||
useEffect(() => {
|
||||
if (props.sliceName) {
|
||||
document.title = props.sliceName;
|
||||
document.title = sanitizeDocumentTitle(props.sliceName);
|
||||
}
|
||||
}, [props.sliceName]);
|
||||
|
||||
|
||||
@@ -243,7 +243,7 @@ test('handles create dashboard button click', async () => {
|
||||
|
||||
const createButton = screen.getByRole('button', { name: /dashboard$/i });
|
||||
await userEvent.click(createButton);
|
||||
expect(assignMock).toHaveBeenCalledWith('/dashboard/new');
|
||||
expect(assignMock).toHaveBeenCalledWith('/dashboard/new/');
|
||||
locationSpy.mockRestore();
|
||||
});
|
||||
|
||||
|
||||
@@ -203,7 +203,7 @@ function DashboardTable({
|
||||
name: t('Dashboard'),
|
||||
buttonStyle: 'secondary',
|
||||
onClick: () => {
|
||||
navigateTo('/dashboard/new', { assign: true });
|
||||
navigateTo('/dashboard/new/', { assign: true });
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -57,7 +57,7 @@ const LABELS = {
|
||||
const REDIRECTS = {
|
||||
create: {
|
||||
[WelcomeTable.Charts]: '/chart/add',
|
||||
[WelcomeTable.Dashboards]: '/dashboard/new',
|
||||
[WelcomeTable.Dashboards]: '/dashboard/new/',
|
||||
// navigateTo() applies the application root internally; keep this
|
||||
// relative so the prefix isn't added twice.
|
||||
[WelcomeTable.SavedQueries]: '/sqllab?new=true',
|
||||
|
||||
@@ -89,7 +89,7 @@ const dropdownItems = [
|
||||
},
|
||||
{
|
||||
label: 'Dashboard',
|
||||
url: '/dashboard/new',
|
||||
url: '/dashboard/new/',
|
||||
icon: 'fa-fw fa-dashboard',
|
||||
perm: 'can_write',
|
||||
view: 'Dashboard',
|
||||
|
||||
@@ -105,7 +105,7 @@ const dropdownItems = [
|
||||
},
|
||||
{
|
||||
label: 'Dashboard',
|
||||
url: '/dashboard/new',
|
||||
url: '/dashboard/new/',
|
||||
icon: 'fa-fw fa-dashboard',
|
||||
perm: 'can_write',
|
||||
view: 'Dashboard',
|
||||
|
||||
@@ -232,7 +232,7 @@ const RightMenu = ({
|
||||
},
|
||||
{
|
||||
label: t('Dashboard'),
|
||||
url: '/dashboard/new',
|
||||
url: '/dashboard/new/',
|
||||
icon: (
|
||||
<Icons.DashboardOutlined data-test={`menu-item-${t('Dashboard')}`} />
|
||||
),
|
||||
|
||||
@@ -762,7 +762,7 @@ function DashboardList(props: DashboardListProps) {
|
||||
name: t('Dashboard'),
|
||||
buttonStyle: 'primary',
|
||||
onClick: () => {
|
||||
navigateTo('/dashboard/new', { assign: true });
|
||||
navigateTo('/dashboard/new/', { assign: true });
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
35
superset-frontend/src/utils/sanitizeDocumentTitle.test.ts
Normal file
35
superset-frontend/src/utils/sanitizeDocumentTitle.test.ts
Normal file
@@ -0,0 +1,35 @@
|
||||
/**
|
||||
* 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 { sanitizeDocumentTitle } from './sanitizeDocumentTitle';
|
||||
|
||||
test('removes all C0 control characters including tab/LF/CR', () => {
|
||||
expect(sanitizeDocumentTitle('a\x08b')).toBe('ab');
|
||||
expect(sanitizeDocumentTitle('x\x09y')).toBe('xy');
|
||||
expect(sanitizeDocumentTitle('x\ny')).toBe('xy');
|
||||
expect(sanitizeDocumentTitle('x\ry')).toBe('xy');
|
||||
});
|
||||
|
||||
test('removes DEL and C1 controls', () => {
|
||||
expect(sanitizeDocumentTitle('a\x7Fb')).toBe('ab');
|
||||
expect(sanitizeDocumentTitle('a\x9Fb')).toBe('ab');
|
||||
});
|
||||
|
||||
test('leaves normal text unchanged', () => {
|
||||
expect(sanitizeDocumentTitle('Dashboard 你好')).toBe('Dashboard 你好');
|
||||
});
|
||||
27
superset-frontend/src/utils/sanitizeDocumentTitle.ts
Normal file
27
superset-frontend/src/utils/sanitizeDocumentTitle.ts
Normal file
@@ -0,0 +1,27 @@
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Strip all C0/C1 control characters (U+0000–U+001F, U+007F–U+009F).
|
||||
* Headless browsers (Playwright/Chromium) can hang or crash when document.title
|
||||
* contains characters such as U+0008 (backspace).
|
||||
*/
|
||||
export function sanitizeDocumentTitle(title: string): string {
|
||||
return title.replace(/[\x00-\x1F\x7F-\x9F]/g, '');
|
||||
}
|
||||
@@ -16,6 +16,7 @@
|
||||
# under the License.
|
||||
|
||||
import math
|
||||
from typing import Optional
|
||||
|
||||
from flask_babel import lazy_gettext as _
|
||||
|
||||
@@ -200,6 +201,26 @@ class ReportScheduleDataFrameFailedError(CommandException):
|
||||
message = _("Report Schedule execution failed when generating a dataframe.")
|
||||
|
||||
|
||||
class ReportScheduleExecutorNotFoundError(CommandException):
|
||||
"""Raised when the configured report executor user cannot be resolved."""
|
||||
|
||||
# 5xx (not 4xx): a missing executor user is a server-side misconfiguration,
|
||||
# not a malformed client request. The status drives get_logger_from_status,
|
||||
# which marks the Celery task FAILURE for 5xx — keeping the executor problem
|
||||
# visible to ops task-state alerting. A 4xx would log a WARNING and leave the
|
||||
# task non-FAILURE, hiding the misconfiguration from that signal.
|
||||
status = 500
|
||||
|
||||
def __init__(self, username: str = "", exception: Optional[Exception] = None):
|
||||
super().__init__(
|
||||
_(
|
||||
"Report Schedule executor user %(username)s was not found.",
|
||||
username=f'"{username}"' if username else "(unknown)",
|
||||
),
|
||||
exception,
|
||||
)
|
||||
|
||||
|
||||
class ReportScheduleExecuteUnexpectedError(CommandException):
|
||||
message = _("Report Schedule execution got an unexpected error.")
|
||||
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
import logging
|
||||
from collections.abc import Sequence
|
||||
from datetime import datetime, timedelta
|
||||
from typing import Any, Optional, Union
|
||||
from typing import Any, Optional, TYPE_CHECKING, Union
|
||||
from uuid import UUID
|
||||
|
||||
import pandas as pd
|
||||
@@ -37,6 +37,7 @@ from superset.commands.report.exceptions import (
|
||||
ReportScheduleDataFrameFailedError,
|
||||
ReportScheduleDataFrameTimeout,
|
||||
ReportScheduleExecuteUnexpectedError,
|
||||
ReportScheduleExecutorNotFoundError,
|
||||
ReportScheduleNotFoundError,
|
||||
ReportSchedulePreviousWorkingError,
|
||||
ReportScheduleScreenshotFailedError,
|
||||
@@ -77,14 +78,42 @@ from superset.utils import json
|
||||
from superset.utils.core import HeaderDataType, override_user, recipients_string_to_list
|
||||
from superset.utils.csv import get_chart_csv_data, get_chart_dataframe
|
||||
from superset.utils.decorators import logs_context, transaction
|
||||
from superset.utils.file import sanitize_title
|
||||
from superset.utils.pdf import build_pdf_from_screenshots
|
||||
from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot
|
||||
from superset.utils.slack import get_channels_with_search, SlackChannelTypes
|
||||
from superset.utils.urls import get_url_path
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from flask_appbuilder.security.sqla.models import User
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def resolve_executor_user(model: ReportSchedule) -> tuple["User", str]:
|
||||
"""
|
||||
Resolve the executor user for a report schedule.
|
||||
|
||||
Determines the configured executor username via ``get_executor`` and looks up
|
||||
the corresponding user. A deleted/disabled user or a misconfigured
|
||||
``ALERT_REPORTS_EXECUTORS`` makes ``security_manager.find_user`` return
|
||||
``None``; rather than passing ``None`` into the webdriver/auth flow (which
|
||||
fails with an opaque NoneType error), raise a dedicated, actionable error.
|
||||
|
||||
:returns: the ``(user, username)`` pair — the username is returned alongside
|
||||
the user because several call sites log it after resolution.
|
||||
:raises ReportScheduleExecutorNotFoundError: if the executor user is missing.
|
||||
"""
|
||||
_, username = get_executor(
|
||||
executors=app.config["ALERT_REPORTS_EXECUTORS"],
|
||||
model=model,
|
||||
)
|
||||
user = security_manager.find_user(username)
|
||||
if user is None:
|
||||
raise ReportScheduleExecutorNotFoundError(username)
|
||||
return user, username
|
||||
|
||||
|
||||
class BaseReportState:
|
||||
current_states: list[ReportState] = []
|
||||
initial: bool = False
|
||||
@@ -135,9 +164,17 @@ class BaseReportState:
|
||||
Update the report schedule type and channels for all slack recipients to v2.
|
||||
V2 uses ids instead of names for channels.
|
||||
"""
|
||||
# Track each recipient mutated in this pass with its original (type,
|
||||
# config) so a partial failure can revert ALL of them — not just the
|
||||
# loop variable. Restoring the in-memory values to their loaded state
|
||||
# keeps a later commit from persisting a half-migrated set.
|
||||
mutated: list[tuple[ReportRecipients, ReportRecipientType, str]] = []
|
||||
try:
|
||||
for recipient in self._report_schedule.recipients:
|
||||
if recipient.type == ReportRecipientType.SLACK:
|
||||
mutated.append(
|
||||
(recipient, recipient.type, recipient.recipient_config_json)
|
||||
)
|
||||
recipient.type = ReportRecipientType.SLACKV2
|
||||
slack_recipients = json.loads(recipient.recipient_config_json)
|
||||
# V1 method allowed to use leading `#` in the channel name
|
||||
@@ -169,8 +206,15 @@ class BaseReportState:
|
||||
}
|
||||
)
|
||||
except Exception as ex:
|
||||
# Revert to v1 to preserve configuration (requires manual fix)
|
||||
recipient.type = ReportRecipientType.SLACK
|
||||
# Revert every mutated recipient to v1 (both type AND config) to
|
||||
# preserve configuration (requires manual fix). Reverting the full
|
||||
# set — not just the loop variable — keeps earlier recipients
|
||||
# consistent; iterating the snapshot also avoids the UnboundLocalError
|
||||
# that a bare loop-variable reference raises on a pre-iteration
|
||||
# failure (which would mask the real error).
|
||||
for reverted_recipient, original_type, original_config in mutated:
|
||||
reverted_recipient.type = original_type
|
||||
reverted_recipient.recipient_config_json = original_config
|
||||
msg = f"Failed to update slack recipients to v2: {str(ex)}"
|
||||
logger.exception(msg)
|
||||
raise UpdateFailedError(msg) from ex
|
||||
@@ -422,11 +466,7 @@ class BaseReportState:
|
||||
"""
|
||||
start_time = datetime.utcnow()
|
||||
|
||||
_, username = get_executor(
|
||||
executors=app.config["ALERT_REPORTS_EXECUTORS"],
|
||||
model=self._report_schedule,
|
||||
)
|
||||
user = security_manager.find_user(username)
|
||||
user, _ = resolve_executor_user(self._report_schedule)
|
||||
|
||||
max_width = app.config["ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH"]
|
||||
|
||||
@@ -512,11 +552,7 @@ class BaseReportState:
|
||||
def _get_csv_data(self) -> bytes:
|
||||
start_time = datetime.utcnow()
|
||||
url = self._get_url(result_format=ChartDataResultFormat.CSV)
|
||||
_, username = get_executor(
|
||||
executors=app.config["ALERT_REPORTS_EXECUTORS"],
|
||||
model=self._report_schedule,
|
||||
)
|
||||
user = security_manager.find_user(username)
|
||||
user, username = resolve_executor_user(self._report_schedule)
|
||||
auth_cookies = machine_auth_provider_factory.instance.get_auth_cookies(user)
|
||||
|
||||
if self._report_schedule.chart.query_context is None:
|
||||
@@ -566,11 +602,7 @@ class BaseReportState:
|
||||
start_time = datetime.utcnow()
|
||||
|
||||
url = self._get_url(result_format=ChartDataResultFormat.JSON)
|
||||
_, username = get_executor(
|
||||
executors=app.config["ALERT_REPORTS_EXECUTORS"],
|
||||
model=self._report_schedule,
|
||||
)
|
||||
user = security_manager.find_user(username)
|
||||
user, username = resolve_executor_user(self._report_schedule)
|
||||
auth_cookies = machine_auth_provider_factory.instance.get_auth_cookies(user)
|
||||
|
||||
if self._report_schedule.chart.query_context is None:
|
||||
@@ -701,7 +733,7 @@ class BaseReportState:
|
||||
error_text = "Unexpected missing csv file"
|
||||
if error_text:
|
||||
return NotificationContent(
|
||||
name=self._report_schedule.name,
|
||||
name=sanitize_title(self._report_schedule.name),
|
||||
text=error_text,
|
||||
header_data=header_data,
|
||||
url=url,
|
||||
@@ -714,15 +746,15 @@ class BaseReportState:
|
||||
embedded_data = self._get_embedded_data()
|
||||
|
||||
if self._report_schedule.email_subject:
|
||||
name = self._report_schedule.email_subject
|
||||
name = sanitize_title(self._report_schedule.email_subject)
|
||||
else:
|
||||
if self._report_schedule.chart:
|
||||
name = (
|
||||
name = sanitize_title(
|
||||
f"{self._report_schedule.name}: "
|
||||
f"{self._report_schedule.chart.slice_name}"
|
||||
)
|
||||
else:
|
||||
name = (
|
||||
name = sanitize_title(
|
||||
f"{self._report_schedule.name}: "
|
||||
f"{self._report_schedule.dashboard.dashboard_title}"
|
||||
)
|
||||
@@ -821,7 +853,7 @@ class BaseReportState:
|
||||
self._execution_id,
|
||||
)
|
||||
notification_content = NotificationContent(
|
||||
name=name, text=message, header_data=header_data, url=url
|
||||
name=sanitize_title(name), text=message, header_data=header_data, url=url
|
||||
)
|
||||
|
||||
# filter recipients to recipients who are also owners
|
||||
@@ -1175,6 +1207,18 @@ class AsyncExecuteReportScheduleCommand(BaseCommand):
|
||||
if not self._model:
|
||||
raise ReportScheduleExecuteUnexpectedError()
|
||||
|
||||
# Resolve the executor at the run() boundary the same way master
|
||||
# does: tolerate a missing user (find_user -> None) so the state
|
||||
# machine still runs and its error envelope writes the ERROR
|
||||
# execution-log row and sends the owner notification. The dedicated
|
||||
# ReportScheduleExecutorNotFoundError guard lives at the content
|
||||
# sites (_get_screenshots / _get_csv_data / _get_embedded_data),
|
||||
# which raise inside that envelope. Guarding here instead would
|
||||
# surface the executor error above the state machine, suppressing
|
||||
# both the log row and the owner notification. The alert-query path
|
||||
# (AlertCommand) is intentionally left on master behavior — a
|
||||
# missing executor there surfaces as a query error, not the
|
||||
# dedicated executor error; tightening it is out of scope here.
|
||||
_, username = get_executor(
|
||||
executors=app.config["ALERT_REPORTS_EXECUTORS"],
|
||||
model=self._model,
|
||||
|
||||
@@ -1979,7 +1979,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
if offset_metrics_df.empty:
|
||||
offset_metrics_df = pd.DataFrame(
|
||||
{
|
||||
col: [np.NaN]
|
||||
col: [np.nan]
|
||||
for col in join_keys + list(metrics_mapping.values())
|
||||
}
|
||||
)
|
||||
|
||||
@@ -128,7 +128,21 @@ class WebhookNotification(BaseNotification):
|
||||
raise NotificationParamException("Webhook URL target host is not allowed.")
|
||||
|
||||
@backoff.on_exception(
|
||||
backoff.expo, NotificationUnprocessableException, factor=10, base=2, max_tries=5
|
||||
backoff.expo,
|
||||
NotificationUnprocessableException,
|
||||
factor=10,
|
||||
base=2,
|
||||
max_tries=5,
|
||||
# Bound total wall-clock retry time. Without this, a hanging or
|
||||
# persistently failing target can stall a worker for minutes per bad URL
|
||||
# (up to ~5 socket waits at timeout=60 plus ~150s of retry sleeps),
|
||||
# starving sequential report dispatch. backoff evaluates max_time against
|
||||
# the elapsed time sampled BEFORE each attempt, so the last request can
|
||||
# start just under the bound and still run its full timeout: worst case
|
||||
# ~210s (~150s elapsed at the final pre-attempt check + one 60s request),
|
||||
# not 120s. factor is intentionally kept at 10 so legitimately-transient
|
||||
# 5xx targets are not abandoned early.
|
||||
max_time=120,
|
||||
)
|
||||
@statsd_gauge("reports.webhook.send")
|
||||
def send(self) -> None:
|
||||
|
||||
@@ -14,10 +14,23 @@
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
import re
|
||||
|
||||
from werkzeug.utils import secure_filename
|
||||
|
||||
# All C0 (U+0000–U+001F) and C1 (U+007F–U+009F) control characters.
|
||||
# Stripping every control char (including tab, LF, CR) keeps titles safe for
|
||||
# SMTP headers, Content-Disposition filenames, and headless-browser document.title.
|
||||
_CONTROL_CHARS_RE = re.compile(r"[\x00-\x1f\x7f-\x9f]")
|
||||
|
||||
|
||||
def sanitize_title(title: str) -> str:
|
||||
"""Remove all C0/C1 control characters from a title string."""
|
||||
return _CONTROL_CHARS_RE.sub("", title)
|
||||
|
||||
|
||||
def get_filename(model_name: str, model_id: int, skip_id: bool = False) -> str:
|
||||
model_name = sanitize_title(model_name)
|
||||
slug = secure_filename(model_name)
|
||||
filename = slug if skip_id else f"{slug}_{model_id}"
|
||||
return filename if slug else str(model_id)
|
||||
|
||||
@@ -71,13 +71,21 @@ def _prophet_fit_and_predict( # pylint: disable=too-many-arguments
|
||||
)
|
||||
if df["ds"].dt.tz:
|
||||
df["ds"] = df["ds"].dt.tz_convert(None)
|
||||
model.fit(df)
|
||||
future = model.make_future_dataframe(periods=periods, freq=freq)
|
||||
forecast = model.predict(future)[["ds", "yhat", "yhat_lower", "yhat_upper"]]
|
||||
try:
|
||||
model.fit(df)
|
||||
future = model.make_future_dataframe(periods=periods, freq=freq)
|
||||
forecast = model.predict(future)[["ds", "yhat", "yhat_lower", "yhat_upper"]]
|
||||
except Exception as ex: # noqa: BLE001
|
||||
raise InvalidPostProcessingError(
|
||||
_(
|
||||
"Unable to generate forecast: %(error)s",
|
||||
error=str(ex),
|
||||
)
|
||||
) from ex
|
||||
return forecast.join(df.set_index("ds"), on="ds").set_index(["ds"])
|
||||
|
||||
|
||||
def prophet( # pylint: disable=too-many-arguments
|
||||
def prophet( # pylint: disable=too-many-arguments # noqa: C901
|
||||
df: DataFrame,
|
||||
time_grain: str,
|
||||
periods: int,
|
||||
@@ -136,6 +144,8 @@ def prophet( # pylint: disable=too-many-arguments
|
||||
raise InvalidPostProcessingError(_("DataFrame must include temporal column"))
|
||||
if len(df.columns) < 2:
|
||||
raise InvalidPostProcessingError(_("DataFrame include at least one series"))
|
||||
if len(df) < 2:
|
||||
raise InvalidPostProcessingError(_("Forecast requires at least 2 data points"))
|
||||
|
||||
target_df = DataFrame()
|
||||
|
||||
|
||||
@@ -26,6 +26,8 @@ from pandas import DataFrame, NamedAgg
|
||||
from superset.constants import TimeGrain
|
||||
from superset.exceptions import InvalidPostProcessingError
|
||||
|
||||
_PANDAS_VERSION = tuple(int(x) for x in pd.__version__.split(".")[:2])
|
||||
|
||||
NUMPY_FUNCTIONS: dict[str, Callable[..., Any]] = {
|
||||
"average": np.average,
|
||||
"argmin": np.argmin,
|
||||
@@ -76,18 +78,18 @@ ALLOWLIST_CUMULATIVE_FUNCTIONS = (
|
||||
)
|
||||
|
||||
PROPHET_TIME_GRAIN_MAP: dict[str, str] = {
|
||||
TimeGrain.SECOND: "S",
|
||||
TimeGrain.SECOND: "s",
|
||||
TimeGrain.MINUTE: "min",
|
||||
TimeGrain.FIVE_MINUTES: "5min",
|
||||
TimeGrain.TEN_MINUTES: "10min",
|
||||
TimeGrain.FIFTEEN_MINUTES: "15min",
|
||||
TimeGrain.THIRTY_MINUTES: "30min",
|
||||
TimeGrain.HOUR: "H",
|
||||
TimeGrain.HOUR: "h",
|
||||
TimeGrain.DAY: "D",
|
||||
TimeGrain.WEEK: "W",
|
||||
TimeGrain.MONTH: "M",
|
||||
TimeGrain.QUARTER: "Q",
|
||||
TimeGrain.YEAR: "A",
|
||||
TimeGrain.MONTH: "ME" if _PANDAS_VERSION >= (2, 2) else "M",
|
||||
TimeGrain.QUARTER: "QE" if _PANDAS_VERSION >= (2, 2) else "Q",
|
||||
TimeGrain.YEAR: "YE" if _PANDAS_VERSION >= (2, 2) else "A",
|
||||
TimeGrain.WEEK_STARTING_SUNDAY: "W-SUN",
|
||||
TimeGrain.WEEK_STARTING_MONDAY: "W-MON",
|
||||
TimeGrain.WEEK_ENDING_SATURDAY: "W-SAT",
|
||||
|
||||
@@ -28,6 +28,7 @@ from superset.commands.exceptions import UpdateFailedError
|
||||
from superset.commands.report.exceptions import (
|
||||
ReportScheduleAlertGracePeriodError,
|
||||
ReportScheduleCsvFailedError,
|
||||
ReportScheduleExecutorNotFoundError,
|
||||
ReportSchedulePreviousWorkingError,
|
||||
ReportScheduleScreenshotFailedError,
|
||||
ReportScheduleScreenshotTimeout,
|
||||
@@ -1096,6 +1097,103 @@ def test_screenshot_width_calculation(
|
||||
)
|
||||
|
||||
|
||||
def _executor_report_state(mocker: MockerFixture) -> BaseReportState:
|
||||
report_schedule = create_report_schedule(mocker)
|
||||
# _get_csv_data/_get_embedded_data build a chart-data URL from chart_id
|
||||
# before resolving the executor; give it a concrete value so URL building
|
||||
# succeeds and the executor resolution is actually reached.
|
||||
report_schedule.chart_id = 1
|
||||
report_schedule.force_screenshot = False
|
||||
return BaseReportState(
|
||||
report_schedule=report_schedule,
|
||||
scheduled_dttm=datetime.now(),
|
||||
execution_id=UUID("084e7ee6-5557-4ecd-9632-b7f39c9ec524"),
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"method_name",
|
||||
["_get_screenshots", "_get_csv_data", "_get_embedded_data"],
|
||||
)
|
||||
def test_get_content_raises_when_executor_user_missing(
|
||||
app: SupersetApp, mocker: MockerFixture, method_name: str
|
||||
) -> None:
|
||||
"""
|
||||
When the configured executor user cannot be resolved
|
||||
(``security_manager.find_user`` returns ``None``), each content path raises a
|
||||
dedicated ``ReportScheduleExecutorNotFoundError`` naming the username at the
|
||||
resolution boundary, rather than passing ``None`` further into the
|
||||
webdriver/auth flow.
|
||||
"""
|
||||
app.config.update(
|
||||
{
|
||||
"ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH": 1600,
|
||||
"WEBDRIVER_WINDOW": {"slice": (800, 600), "dashboard": (800, 600)},
|
||||
"ALERT_REPORTS_EXECUTORS": {},
|
||||
}
|
||||
)
|
||||
report_state = _executor_report_state(mocker)
|
||||
|
||||
with (
|
||||
patch("superset.commands.report.execute.security_manager") as mock_sm,
|
||||
patch("superset.commands.report.execute.get_executor") as mock_get_executor,
|
||||
patch("superset.commands.report.execute.machine_auth_provider_factory"),
|
||||
):
|
||||
mock_get_executor.return_value = ("executor", "ghost_user")
|
||||
mock_sm.find_user = mocker.MagicMock(return_value=None)
|
||||
|
||||
with pytest.raises(ReportScheduleExecutorNotFoundError, match="ghost_user"):
|
||||
getattr(report_state, method_name)()
|
||||
|
||||
|
||||
def test_executor_not_found_error_message_without_username() -> None:
|
||||
"""
|
||||
When no username is available, the message falls back to ``(unknown)``
|
||||
rather than leaving a double space ("...executor user was not found.").
|
||||
"""
|
||||
message = str(ReportScheduleExecutorNotFoundError().message)
|
||||
|
||||
assert "(unknown)" in message
|
||||
assert "user was" not in message
|
||||
|
||||
|
||||
def test_executor_not_found_error_status_is_server_error() -> None:
|
||||
"""
|
||||
The executor-not-found error is a 5xx so ``get_logger_from_status`` marks the
|
||||
Celery task ``FAILURE`` (a missing executor is a server-side misconfiguration
|
||||
that ops task-state alerting must still see), not a 4xx that would log a
|
||||
``WARNING`` and leave the task non-``FAILURE``.
|
||||
"""
|
||||
assert ReportScheduleExecutorNotFoundError().status == 500
|
||||
|
||||
|
||||
def test_resolve_executor_user_returns_user_and_username(
|
||||
app: SupersetApp, mocker: MockerFixture
|
||||
) -> None:
|
||||
"""
|
||||
Happy path: when the executor user exists, the helper returns the
|
||||
``(user, username)`` tuple unchanged — locking the no-behavior-change exit
|
||||
criterion for the three call sites.
|
||||
"""
|
||||
from superset.commands.report.execute import resolve_executor_user
|
||||
|
||||
app.config.update({"ALERT_REPORTS_EXECUTORS": {}})
|
||||
report_schedule = create_report_schedule(mocker)
|
||||
mock_user = mocker.MagicMock()
|
||||
|
||||
with (
|
||||
patch("superset.commands.report.execute.security_manager") as mock_sm,
|
||||
patch("superset.commands.report.execute.get_executor") as mock_get_executor,
|
||||
):
|
||||
mock_get_executor.return_value = ("executor", "real_user")
|
||||
mock_sm.find_user = mocker.MagicMock(return_value=mock_user)
|
||||
|
||||
user, username = resolve_executor_user(report_schedule)
|
||||
|
||||
assert user is mock_user
|
||||
assert username == "real_user"
|
||||
|
||||
|
||||
def test_update_recipient_to_slack_v2(mocker: MockerFixture):
|
||||
"""
|
||||
Test converting a Slack recipient to Slack v2 format.
|
||||
@@ -1171,6 +1269,122 @@ def test_update_recipient_to_slack_v2_missing_channels(mocker: MockerFixture):
|
||||
mock_cmmd.update_report_schedule_slack_v2()
|
||||
|
||||
|
||||
def test_update_recipient_to_slack_v2_reverts_all_on_partial_failure(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
When the second of two Slack recipients fails channel resolution, BOTH
|
||||
recipients are fully reverted — type AND exact original
|
||||
``recipient_config_json`` string — not just the loop variable's type. This
|
||||
prevents the intervening ``create_log`` commit from flushing a half-migrated,
|
||||
inconsistent state.
|
||||
"""
|
||||
|
||||
def channels_side_effect(search_string, types, exact_match):
|
||||
if search_string == "Channel-1":
|
||||
return [
|
||||
{
|
||||
"id": "id_channel_1",
|
||||
"name": "Channel-1",
|
||||
"is_member": True,
|
||||
"is_private": False,
|
||||
}
|
||||
]
|
||||
# Second recipient: no channel found → length mismatch → UpdateFailedError
|
||||
return []
|
||||
|
||||
mocker.patch(
|
||||
"superset.commands.report.execute.get_channels_with_search",
|
||||
side_effect=channels_side_effect,
|
||||
)
|
||||
original_config_1 = json.dumps({"target": "Channel-1"})
|
||||
original_config_2 = json.dumps({"target": "Channel-2"})
|
||||
mock_report_schedule = ReportSchedule(
|
||||
name="Test Report",
|
||||
recipients=[
|
||||
ReportRecipients(
|
||||
type=ReportRecipientType.SLACK,
|
||||
recipient_config_json=original_config_1,
|
||||
),
|
||||
ReportRecipients(
|
||||
type=ReportRecipientType.SLACK,
|
||||
recipient_config_json=original_config_2,
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
mock_cmmd = BaseReportState(
|
||||
mock_report_schedule, "January 1, 2021", "execution_id_example"
|
||||
)
|
||||
|
||||
with pytest.raises(UpdateFailedError):
|
||||
mock_cmmd.update_report_schedule_slack_v2()
|
||||
|
||||
first, second = mock_report_schedule.recipients
|
||||
# The first recipient was mutated to v2 before the second failed; it must be
|
||||
# reverted to its exact original type AND config string.
|
||||
assert first.type == ReportRecipientType.SLACK
|
||||
assert first.recipient_config_json == original_config_1
|
||||
assert second.type == ReportRecipientType.SLACK
|
||||
assert second.recipient_config_json == original_config_2
|
||||
|
||||
|
||||
def test_update_recipient_to_slack_v2_pre_iteration_failure(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
A failure raised while accessing/iterating the recipients (before the loop
|
||||
variable is bound) surfaces as ``UpdateFailedError``, not a ``NameError``
|
||||
that would mask the real error.
|
||||
"""
|
||||
|
||||
class _ExplodingRecipients:
|
||||
def __iter__(self):
|
||||
raise RuntimeError("recipients exploded")
|
||||
|
||||
mock_report_schedule = mocker.MagicMock()
|
||||
mock_report_schedule.recipients = _ExplodingRecipients()
|
||||
|
||||
mock_cmmd = BaseReportState(
|
||||
mock_report_schedule, "January 1, 2021", "execution_id_example"
|
||||
)
|
||||
|
||||
with pytest.raises(UpdateFailedError):
|
||||
mock_cmmd.update_report_schedule_slack_v2()
|
||||
|
||||
|
||||
def test_update_recipient_to_slack_v2_no_slack_recipients_is_noop(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
With no SLACK recipients there is nothing to migrate: the method returns
|
||||
without raising and leaves the non-Slack recipients untouched.
|
||||
"""
|
||||
mock_search = mocker.patch(
|
||||
"superset.commands.report.execute.get_channels_with_search",
|
||||
)
|
||||
mock_report_schedule = ReportSchedule(
|
||||
recipients=[
|
||||
ReportRecipients(
|
||||
type=ReportRecipientType.EMAIL,
|
||||
recipient_config_json=json.dumps({"target": "user@example.com"}),
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
mock_cmmd: BaseReportState = BaseReportState(
|
||||
mock_report_schedule, "January 1, 2021", "execution_id_example"
|
||||
)
|
||||
mock_cmmd.update_report_schedule_slack_v2()
|
||||
|
||||
assert mock_cmmd._report_schedule.recipients[0].type == ReportRecipientType.EMAIL
|
||||
assert (
|
||||
mock_cmmd._report_schedule.recipients[0].recipient_config_json
|
||||
== '{"target": "user@example.com"}'
|
||||
)
|
||||
mock_search.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tier 1: _update_query_context + create_log
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
# under the License.
|
||||
from datetime import datetime
|
||||
from importlib.util import find_spec
|
||||
from unittest.mock import patch
|
||||
|
||||
import pandas as pd
|
||||
import pytest
|
||||
@@ -186,6 +187,43 @@ def test_prophet_incorrect_time_grain():
|
||||
)
|
||||
|
||||
|
||||
def test_prophet_insufficient_data():
|
||||
single_row_df = pd.DataFrame(
|
||||
{
|
||||
DTTM_ALIAS: [datetime(2022, 1, 1)],
|
||||
"sales": [100.0],
|
||||
}
|
||||
)
|
||||
with pytest.raises(InvalidPostProcessingError, match="at least 2 data points"):
|
||||
prophet(
|
||||
df=single_row_df,
|
||||
time_grain="P1D",
|
||||
periods=3,
|
||||
confidence_interval=0.9,
|
||||
)
|
||||
|
||||
|
||||
def test_prophet_fit_error():
|
||||
if find_spec("prophet") is None:
|
||||
pytest.skip("prophet not installed")
|
||||
|
||||
with patch(
|
||||
"superset.utils.pandas_postprocessing.prophet._prophet_fit_and_predict"
|
||||
) as mock_fit:
|
||||
mock_fit.side_effect = InvalidPostProcessingError(
|
||||
"Unable to generate forecast: Dataframe has fewer than 2 non-NaN rows."
|
||||
)
|
||||
with pytest.raises(
|
||||
InvalidPostProcessingError, match="Unable to generate forecast"
|
||||
):
|
||||
prophet(
|
||||
df=prophet_df,
|
||||
time_grain="P1D",
|
||||
periods=3,
|
||||
confidence_interval=0.9,
|
||||
)
|
||||
|
||||
|
||||
def test_prophet_uncertainty_lower_bound_can_be_negative_for_negative_series():
|
||||
"""
|
||||
Regression for #21734: when the input series contains negative values,
|
||||
|
||||
@@ -18,9 +18,11 @@
|
||||
|
||||
import pandas as pd
|
||||
import pytest
|
||||
from freezegun import freeze_time
|
||||
|
||||
from superset.reports.notifications.exceptions import (
|
||||
NotificationParamException,
|
||||
NotificationUnprocessableException,
|
||||
)
|
||||
from superset.reports.notifications.webhook import WebhookNotification
|
||||
from superset.utils.core import HeaderDataType
|
||||
@@ -269,3 +271,156 @@ def test_send_treats_redirect_as_failure(monkeypatch, mock_header_data) -> None:
|
||||
|
||||
with pytest.raises(NotificationParamException, match="redirect"):
|
||||
webhook_notification.send()
|
||||
|
||||
|
||||
def _make_webhook(mock_header_data) -> WebhookNotification:
|
||||
from superset.reports.models import ReportRecipients, ReportRecipientType
|
||||
from superset.reports.notifications.base import NotificationContent
|
||||
|
||||
content = NotificationContent(
|
||||
name="test alert", header_data=mock_header_data, description="Test description"
|
||||
)
|
||||
return WebhookNotification(
|
||||
recipient=ReportRecipients(
|
||||
type=ReportRecipientType.WEBHOOK,
|
||||
recipient_config_json='{"target": "https://example.com/webhook"}',
|
||||
),
|
||||
content=content,
|
||||
)
|
||||
|
||||
|
||||
class _MockServerErrorResponse:
|
||||
status_code = 500
|
||||
text = ""
|
||||
|
||||
|
||||
def _allow_internal_app() -> type:
|
||||
class MockCurrentApp:
|
||||
config = {
|
||||
"ALERT_REPORTS_WEBHOOK_HTTPS_ONLY": True,
|
||||
"ALERT_REPORTS_WEBHOOK_ALLOW_INTERNAL_HOSTS": True,
|
||||
}
|
||||
|
||||
return MockCurrentApp
|
||||
|
||||
|
||||
def test_send_backoff_bounded_by_max_time(monkeypatch, mock_header_data) -> None:
|
||||
"""
|
||||
A persistently failing (500) target gives up on wall-time (``max_time``),
|
||||
not just ``max_tries``. ``freeze_time(auto_tick_seconds=30)`` advances the
|
||||
clock on every time access (backoff measures elapsed via ``datetime.now``,
|
||||
which freezegun freezes and ticks), so cumulative elapsed crosses ``max_time=120``
|
||||
before ``max_tries=5`` is exhausted. We assert the discriminating *property*
|
||||
— gave up on wall-time strictly before exhausting ``max_tries`` — rather than
|
||||
a pinned count, because freezegun's per-call tick count is an opaque
|
||||
implementation detail. If ``max_time`` is removed this rises to the full 5
|
||||
(RED); the flat-clock companion test below anchors that 5 is the ceiling.
|
||||
The terminal exception type is unchanged on giveup.
|
||||
"""
|
||||
webhook_notification = _make_webhook(mock_header_data)
|
||||
post_calls: list[int] = []
|
||||
|
||||
def fake_post(*args, **kwargs) -> _MockServerErrorResponse:
|
||||
post_calls.append(1)
|
||||
return _MockServerErrorResponse()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.current_app", _allow_internal_app()
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.feature_flag_manager.is_feature_enabled",
|
||||
lambda flag: True,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.requests.post", fake_post
|
||||
)
|
||||
monkeypatch.setattr("backoff._sync.time.sleep", lambda *a, **k: None)
|
||||
|
||||
with freeze_time("2020-01-01", auto_tick_seconds=30):
|
||||
with pytest.raises(NotificationUnprocessableException):
|
||||
webhook_notification.send()
|
||||
|
||||
# 1 < count < max_tries(5): wall-time bound fired before tries exhausted.
|
||||
assert 1 < len(post_calls) < 5
|
||||
|
||||
|
||||
def test_send_flat_clock_falls_back_to_max_tries(monkeypatch, mock_header_data) -> None:
|
||||
"""
|
||||
Characterization (NOT a RED discriminator): with the clock held flat,
|
||||
``max_time`` can never fire, so ``max_tries=5`` governs and exactly 5 POSTs
|
||||
happen. Passes on both buggy and fixed code; its job is to prove the 3-vs-5
|
||||
delta in ``test_send_backoff_bounded_by_max_time`` is attributable to
|
||||
wall-time, not to ``max_tries``.
|
||||
"""
|
||||
webhook_notification = _make_webhook(mock_header_data)
|
||||
post_calls: list[int] = []
|
||||
|
||||
def fake_post(*args, **kwargs) -> _MockServerErrorResponse:
|
||||
post_calls.append(1)
|
||||
return _MockServerErrorResponse()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.current_app", _allow_internal_app()
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.feature_flag_manager.is_feature_enabled",
|
||||
lambda flag: True,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.requests.post", fake_post
|
||||
)
|
||||
monkeypatch.setattr("backoff._sync.time.sleep", lambda *a, **k: None)
|
||||
|
||||
with freeze_time("2020-01-01"):
|
||||
with pytest.raises(NotificationUnprocessableException):
|
||||
webhook_notification.send()
|
||||
|
||||
assert len(post_calls) == 5
|
||||
|
||||
|
||||
def test_send_max_time_does_not_abandon_recovering_target(
|
||||
monkeypatch, mock_header_data
|
||||
) -> None:
|
||||
"""
|
||||
No-regression guard: a target that fails twice (500) then succeeds on the
|
||||
3rd attempt still succeeds, and ``max_time=120`` is not set so low it
|
||||
abandons recovery. The clock advances a small +5s per access so ``max_time``
|
||||
is genuinely engaged (cumulative elapsed across 3 attempts stays comfortably
|
||||
under 120), unlike a flat clock which would pin elapsed at 0 and let the test
|
||||
pass at any ``max_time`` — including a misconfigured ``max_time=1``. Lower
|
||||
``max_time`` enough and this test correctly fails.
|
||||
"""
|
||||
webhook_notification = _make_webhook(mock_header_data)
|
||||
post_calls: list[int] = []
|
||||
|
||||
class _OkResponse:
|
||||
status_code = 200
|
||||
text = ""
|
||||
|
||||
def fake_post(*args, **kwargs):
|
||||
post_calls.append(1)
|
||||
if len(post_calls) < 3:
|
||||
return _MockServerErrorResponse()
|
||||
return _OkResponse()
|
||||
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.current_app", _allow_internal_app()
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.feature_flag_manager.is_feature_enabled",
|
||||
lambda flag: True,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"superset.reports.notifications.webhook.requests.post", fake_post
|
||||
)
|
||||
monkeypatch.setattr("backoff._sync.time.sleep", lambda *a, **k: None)
|
||||
|
||||
with freeze_time("2020-01-01", auto_tick_seconds=5):
|
||||
webhook_notification.send()
|
||||
|
||||
# The clock advances (+5s/access) so ``max_time`` is genuinely engaged, yet
|
||||
# cumulative elapsed across 3 attempts stays well under 120: the recovering
|
||||
# target (``fake_post`` succeeds on the 3rd) is not abandoned. A flat clock
|
||||
# would pin elapsed at 0 and pass at any ``max_time``, including a broken
|
||||
# ``max_time=1`` — this non-zero tick keeps the guard real.
|
||||
assert len(post_calls) == 3
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
# under the License.
|
||||
import pytest
|
||||
|
||||
from superset.utils.file import get_filename
|
||||
from superset.utils.file import get_filename, sanitize_title
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
@@ -34,6 +34,9 @@ from superset.utils.file import get_filename
|
||||
("你好", 475, True, "475"),
|
||||
("Energy Sankey 你好", 475, False, "Energy_Sankey_475"),
|
||||
("Energy Sankey 你好", 475, True, "Energy_Sankey"),
|
||||
("Energy\x08Sankey", 132, False, "EnergySankey_132"),
|
||||
("Energy\x08Sankey", 132, True, "EnergySankey"),
|
||||
("Sales\x7fReport", 1, False, "SalesReport_1"),
|
||||
],
|
||||
)
|
||||
def test_get_filename(
|
||||
@@ -41,3 +44,20 @@ def test_get_filename(
|
||||
) -> None:
|
||||
original_filename = get_filename(model_name, model_id, skip_id)
|
||||
assert expected_filename == original_filename
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("raw", "expected"),
|
||||
[
|
||||
("normal", "normal"),
|
||||
("a\x08b", "ab"),
|
||||
("x\x09y", "xy"),
|
||||
("x\ny", "xy"),
|
||||
("x\ry", "xy"),
|
||||
("\x00\x01\x02", ""),
|
||||
("a\x7fb", "ab"),
|
||||
("a\x9fb", "ab"),
|
||||
],
|
||||
)
|
||||
def test_sanitize_title(raw: str, expected: str) -> None:
|
||||
assert sanitize_title(raw) == expected
|
||||
|
||||
Reference in New Issue
Block a user