sadpandajoe commented on code in PR #43303: URL: https://github.com/apache/superset/pull/43303#discussion_r3817802805
########## superset-frontend/scripts/gen-qc-registry.mjs: ########## @@ -0,0 +1,216 @@ +/** + * 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. + */ + +// Deterministic, re-runnable codegen: maps every plugin `buildQuery` module to the +// viz_type key(s) it is registered under, and emits registry.generated.ts consumed +// by entry.ts. Join: buildQuery module <- (index.ts that imports it) -> plugin class +// name -> MainPreset `.configure({ key: VizType.X })` -> VizType enum string. +// A single builder legitimately maps to several keys (e.g. echarts_timeseries + _line/_bar/...). +import { readFileSync, writeFileSync, readdirSync, statSync, existsSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const FE = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const PLUGINS = path.join(FE, 'plugins'); +const OUT = path.join(FE, 'src', 'backend-querycontext', 'registry.generated.ts'); + +// --- 1. VizType enum: EnumName -> 'string_value' --- +const vizTypeSrc = readFileSync( + path.join(FE, 'packages/superset-ui-core/src/chart/types/VizType.ts'), + 'utf8', +); +const VIZ_ENUM = {}; +for (const m of vizTypeSrc.matchAll(/(\w+)\s*=\s*['"]([\w-]+)['"]/g)) VIZ_ENUM[m[1]] = m[2]; + +// --- 2. MainPreset: ClassName -> viz string --- +const mainPreset = readFileSync( + path.join(FE, 'src/visualizations/presets/MainPreset.js'), Review Comment: This generator reads `MainPreset.js`, but the repository only contains `MainPreset.ts`, so regeneration exits with ENOENT and the registry cannot track plugin changes. Should this target the TypeScript source and add a smoke check? ########## superset/commands/chart/importers/v1/utils.py: ########## @@ -70,6 +77,64 @@ def import_chart( filter_chart_annotations(config) + # Synthesize a query_context for imported charts that arrive without one, so + # the first `GET /api/v1/chart/{pk}/data/` returns data instead of HTTP 400 + # "Chart has no query context saved" (issue #33615, ADR-013). Guarded on an + # ABSENT context so an existing/remapped one is never overwritten (FR-006). + # + # Two-tier derivation: + # 1. AUTHORITATIVE — run the chart's real frontend `buildQuery` in V8 + # (QueryContextGenerator) for byte-faithful parity with the UI. + # 2. FALLBACK — a pure-Python generic derivation + # (`build_query_context_config`) when the V8 bundle / py_mini_racer is + # unavailable or the viz type is not (yet) covered by the bundle. + # Either way the datasource is taken from the importer-resolved id/type ONLY, + # never a value carried in params (ADR-014 authz/RLS). A per-chart derivation + # error must never abort the bundle (RISK-T03 / FR-004). + if not config.get("query_context"): + try: + params = config.get("params") or {} + viz_type = config["viz_type"] + datasource_id = config.get("datasource_id") + datasource_type = config.get("datasource_type", "table") + + query_context_config = None + if datasource_id: + # form_data for the JS builder: the datasource is the + # importer-resolved id/type only (overwrite any incoming + # params.datasource — never trust it; ADR-014). + js_params = { + **params, + "datasource": f"{datasource_id}__{datasource_type}", + } + query_context_config = get_query_context_generator().generate( + viz_type, js_params + ) + if query_context_config is None: + query_context_config = build_query_context_config( + params, viz_type, datasource_id, datasource_type Review Comment: Agreed—legacy `dual_line` imports synthesize before migration, then retain a context whose queries do not match the resulting two-series `mixed_timeseries` form data. Should synthesis run after migration or be regenerated when the migration changes the chart? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
