fitzee commented on PR #45530:
URL: https://github.com/apache/superset/pull/45530#issuecomment-6329496445
**Re-checked at `38e286ce`:** LGTM still stands. The only new commit is a
master merge, and it doesn't change this PR's logic.
- **What changed.** `b7db5b62..38e286ce` is one merge commit (`38e286ce`,
master at `bc83e3e6fd`). `git range-diff` against each side's merge-base shows
the PR's 6 commits unchanged (`=`). The diff of the PR against master is
identical apart from line offsets. The merge's only hand resolution is in
`UPDATING.md`: the PR's entry and master's new MCP entries are both kept.
#45528 is also unchanged (head still `0099a023`).
- **Master changes near this code.** None of them affect the copy path:
- `superset/daos/dashboard.py` (#45451) adds an
`EXTRA_ACCESS_QUERY_EXCLUSIONS` check to the uuid branch of
`get_by_id_or_slug`. That is upstream of copy and doesn't touch metadata.
- `superset/versioning/restore.py` (#45482) changes restore membership
only. Copy doesn't call it.
- MCP `duplicate_dashboard` (#45504) now sets `duplicate_slices=True` by
default. `_build_copy_payload` still sends `source.json_metadata` plus
`positions`. The MCP default now runs through the `old_to_new_slice_ids` branch
of `_copy_chart_metadata`, which the PR's tests already cover. The new
`duplicate_slices=None` case in `test_copy_archived_slot_policy_and_restore`
passes with this PR's DAO.
- **#45538 is not merged.** It's still open (head `f9afb4ad`), so this
branch has nothing to resolve yet. A test merge of the two heads still
conflicts:
- `superset/daos/dashboard.py:1111-1113`: same as before, only shifted by
the master merge. Whichever PR merges second must keep `set_dash_metadata(dash,
metadata, old_to_new_slice_ids, allowed_chart_ids=...)` and must not bring back
`dash.params = original_dash.params`.
- `tests/integration_tests/dashboards/api_tests.py`: both PRs add tests at
the same spot. Keep both.
- **Prior nits.**
1. #45538 conflict: still open, as above.
2. `config["id"] = int(chart_key)` at `superset/daos/dashboard.py:404`
(was :403): still not covered by any test.
3. `DashboardJSONMetadataSchema` `unknown=RAISE` on old snapshots:
unchanged. It predates this PR.
- **Tests/CI.** CI on `38e286ce` is green (58 pass, 14 skipped), including
test-postgres, test-mysql, test-sqlite and unit-tests. Locally,
`tests/unit_tests/dao/test_copy_dashboard_legacy_ids.py`, `dashboard_test.py`,
`subjects/test_creator_group_call_sites.py` and
`tests/unit_tests/mcp_service/dashboard` pass at `38e286ce` (1293 passed).
--
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]
