aminghadersohi commented on PR #39922: URL: https://github.com/apache/superset/pull/39922#issuecomment-4558261266
## fitzee review follow-up — 4 blocking items addressed (dc73a0bf) ### 1. `register()` lock protection (HIGH) `register()` now wraps the `_REGISTRY[plugin.chart_type] = plugin` write in `with _plugins_lock:`. The lock was also upgraded from `threading.Lock()` to `threading.RLock()` so plugin `register()` calls made *during* the lazy import inside `_ensure_plugins_loaded()` don't deadlock. ### 2. Column regex removal + PR #39915 relationship (HIGH) Added an explicit **Column-name sanitization and PR #39915** paragraph in ADDITIONAL INFORMATION. It states: this branch was branched before #39915 merged; the regex is intentionally absent because each of the three fields (`ColumnRef.name`, `FilterConfig.column`, `BigNumberChartConfig.temporal_column`) is now sanitized at the schema boundary with `sanitize_user_input(..., check_sql_keywords=True)`. The `check_sql_keywords=True` argument was also added to the `FilterConfig` and `BigNumberChartConfig` validators in dc73a0bf. ### 3. `_ensure_plugins_loaded()` circuit breaker (MEDIUM) `_plugins_load_failed = False` sentinel was already added in e134b8380d. dc73a0bf hardened this further: on import failure, the registry is rolled back to its pre-import snapshot (`registry_before_import = dict(_REGISTRY)`) before setting `_plugins_load_failed = True`, ensuring no partially-registered plugins leak through. New test `test_ensure_plugins_loaded_rolls_back_partial_registration` covers this path. ### 4. 5-type column validation behavioral change (documentation) Added a **Behavioral change — 5-type column validation gap** paragraph in ADDITIONAL INFORMATION. It explains: the bug was found during the refactor (no prior Shortcut/GitHub issue exists); `pie`, `pivot_table`, `mixed_timeseries`, `handlebars`, and `big_number` previously skipped column validation entirely; this PR intentionally makes all 7 types consistent. Callers that relied on silent pass-through will now receive `column_not_found` validation errors. -- 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]
