eschutho opened a new pull request, #43305:
URL: https://github.com/apache/superset/pull/43305

   ### SUMMARY
   
   Catches a raw `KeyError` in `load_configs()` so a database import config 
missing `uuid`/`ssh_tunnel` fails with a clean validation error instead of an 
opaque 500.
   
   #### Problem
   
   `load_configs()` reads `config["uuid"]` and `config["ssh_tunnel"][...]` 
directly before the marshmallow schema validation runs. A database import YAML 
that is missing either key raises a raw `KeyError` that escapes uncaught, 
surfacing as an opaque 500 from the `*/import/` endpoints instead of a clean 
validation error. Confirmed via a git-stash repro against pre-fix code: it 
fails with `KeyError: 'uuid'` at `utils.py:162`.
   
   #### Fix
   
   Added a sibling `except KeyError as exc:` next to the existing `except 
ValidationError as exc:`, converting the failure into an aggregated 
`ValidationError` like every other per-file import failure. This mirrors the 
sibling fix in this same file for `masked_encrypted_extra` JSON decoding (see 
#43261 for context: https://github.com/apache/superset/pull/43261). There is no 
behavior change for well-formed configs.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — backend error-handling change.
   
   ### TESTING INSTRUCTIONS
   
   Two new tests were added to `TestLoadConfigs` in 
`tests/unit_tests/commands/importers/v1/utils_test.py`:
   
   - **Missing-`uuid` case**: a database config with no `uuid` key now raises a 
`ValidationError` (aggregated per-file error) rather than a raw `KeyError`.
   - **Control case**: a well-formed config with `uuid` present still loads 
cleanly with no error.
   
   Run:
   
   ```bash
   pytest tests/unit_tests/commands/importers/v1/utils_test.py
   ```
   
   Also verified locally: `uvx [email protected] check` and `uvx [email protected] format 
--check` are clean, and mypy reports no new errors.
   
   ### ADDITIONAL INFORMATION
   
   **Tradeoffs:** This is an additive catch only — no existing failure-mode 
semantics change for valid imports. Malformed imports that previously returned 
a 500 now return a clean 4xx validation error. This is the intended, additive 
change and matches the sibling PR's pattern; there is no undisclosed semantics 
shift.
   
   <!--- Check any relevant boxes with "x" -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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]

Reply via email to