kokhlo commented on PR #44307: URL: https://github.com/apache/superset/pull/44307#issuecomment-5974336839
@rusackas thanks for catching the empty-slug case — you were right, and it was the `not slug` vs `is not None` asymmetry I described in #44307. `import_dashboard()` resolves a slug collision with `is not None`, so `slug=""` is an identity value there: a fresh-UUID config carrying `slug=""` against a dashboard that already owns one got resolved onto that row, and without `overwrite` the bundle's charts merged into it silently, with no confirmation prompt. My gate skipped it. The branch now exempts only a missing slug, matching the resolution branch and `DashboardDAO.validate_slug_uniqueness` (which uses `slug is None` for the same reason). `test_import_empty_slug_collision_is_flagged` covers it — I checked the new test is what fails on the old code (`DID NOT RAISE`), the other five stay green, so it is not passing by accident. On CI: the red across the board and the alembic multiple-heads sqlite failure are gone on their own — the branch is 0 commits behind master now, and the suite is green except `pre-commit`, which was one mypy error in my own test helper (`Need type annotation for "dashboard"`, `import_test.py:772`); annotated, and mypy is clean on both changed files with the pinned 1.15.0. So no rebase needed on my side — sorry for the noise in the meantime. -- 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]
