eschutho opened a new pull request, #44390: URL: https://github.com/apache/superset/pull/44390
### SUMMARY Fixes an `UnboundLocalError` that turns a per-file YAML validation failure during import into an unhandled 500. **Sentry:** [SUPERSET-PYTHON-176W](https://preset-inc.sentry.io/issues/7738053303/) — `UnboundLocalError: cannot access local variable 'config' where it is not associated with a value`, culprit `ChartRestApi.import_`. 2 events, 1 user. **Shortcut:** https://app.shortcut.com/preset/story/121288 #### Root cause In `load_configs` (`superset/commands/importers/v1/utils.py`), each file is processed as: ```python try: config = load_yaml(file_name, content) ... except ValidationError as exc: ... if isinstance(config, dict): # diagnostic ... ``` When the imported YAML is unparseable, `load_yaml()` raises `ValidationError` **before** the `config = load_yaml(...)` assignment ever completes, so `config` is never bound in that loop iteration. The `except ValidationError` handler then evaluates `isinstance(config, dict)` and crashes with `UnboundLocalError` — an assignment-in-`try` that never completes leaves the diagnostic's own guard unbound. Ironically that guard was added specifically to be crash-proof ("config may be a non-mapping"), but it never handled the "config was never assigned at all" case. #### Fix Bind `config: Any = None` immediately before the `try:` block so the diagnostic guard is always valid. `isinstance(None, dict)` is `False`, so an unparseable file falls into the existing `else` branch and is correctly reported via the `exceptions` list — exactly like every other per-file failure. `Any` is already imported in this module. ### TESTING INSTRUCTIONS - **Repro (pre-fix):** the new regression test `test_unparseable_yaml_is_reported_as_validation_error` fails with `UnboundLocalError: ... 'config' ...` at `utils.py:316` on unpatched code. - **After fix:** the file is reported via `exceptions` (not `configs`), no crash. Full file green: ``` $ pytest tests/unit_tests/commands/importers/v1/utils_test.py -q 20 passed $ ruff check / ruff format --check # both clean on the two touched files ``` ### Tradeoffs None. This only changes how a pre-existing crash is avoided and what gets logged (the unparseable file is now reported as `type: NoneType` via the existing `else` diagnostic branch). No user-facing behavior change beyond no longer returning a 500 — the file is still correctly reported as invalid via `exceptions`. No new imports, no migration, no API change. ### ADDITIONAL INFORMATION - [x] Has associated issue: Fixes SUPERSET-PYTHON-176W - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Introduces new feature or API - [ ] Removes existing feature or API 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
