sadpandajoe opened a new pull request, #41759: URL: https://github.com/apache/superset/pull/41759
### SUMMARY When the `/superset` URL prefix was removed (`Superset.route_base = ""`), a closed-set 308 redirect shim (`superset/middleware/legacy_prefix_redirect.py`) was added to keep the enumerated bookmarkable `/superset/*` routes working for one release cycle. One route was **intentionally excluded**: `/superset/sql/<database_id>/`, because `Database.sql_url` changed shape from a path to a query string (`/sqllab/?dbid=<id>`), so there is no 1:1 path→path mapping for the closed-set map. The side effect is that existing **SQL Lab bookmarks / deep links** of the form `/superset/sql/5/` hard-404 instead of degrading gracefully like every other legacy `/superset/*` route. This PR closes that gap without weakening the closed-set discipline that protects the rest of the map. **Approach** — a dedicated special case (`_LEGACY_SQL_RE = ^/sql/(\d+)/?$`) handled ahead of the closed-set lookup: - A **numeric** `<database_id>` → **308** to `/sqllab/?dbid=<id>`. - Any inbound query string is merged with `&` (so `location` never ends up with `?…?…`). - The redirect respects the deployment **app-root prefix** (subdirectory deploys). - **Non-GET** methods → **410 Gone** (the old route was GET-only; a 308 would make the client retry-POST against `/sqllab/` → 405). - A **non-numeric** tail does *not* match `_LEGACY_SQL_RE` and falls through to the closed-set map (→ 404), so closed-set discipline is preserved for everything that isn't this specific, well-defined transform. This is a deliberate, documented departure from the original "leave it to hard-404" decision: the module docstring is updated to explain why this one route is handled by a special case rather than the map. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Before: `GET /superset/sql/5/` → 404. After: `GET /superset/sql/5/` → `308` → `/sqllab/?dbid=5`. ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/middleware/test_legacy_prefix_redirect.py ``` Added regression cases cover: numeric id → 308 to `/sqllab/?dbid=<id>`; query-string `&`-merge; app-root prefix; non-GET → 410; non-numeric tail falling through to the closed-set map (404). ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] 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]
