mikebridge commented on code in PR #43724:
URL: https://github.com/apache/superset/pull/43724#discussion_r3897550207
##########
superset-frontend/src/pages/ArchivedList/index.tsx:
##########
@@ -286,6 +301,123 @@ function ArchivedListBody({
[performRowAction, addSuccessToast],
);
+ const loadDatasetPurgeImpact = useCallback(async (item: ArchivedItem) => {
+ const generation = impactRequestGeneration.current + 1;
+ impactRequestGeneration.current = generation;
+ setDatasetPurgeModal({ status: 'loading', item });
+
+ try {
+ const { json } = await SupersetClient.get({
+ endpoint: `/api/v1/dataset/${item.uuid}/purge-impact`,
+ });
+ if (impactRequestGeneration.current !== generation) {
+ return;
+ }
+ setDatasetPurgeModal({
+ status: 'ready',
+ item,
+ impact: json as PurgeImpactResponse,
+ });
+ } catch (error) {
+ if (impactRequestGeneration.current !== generation) {
+ return;
+ }
+ const { error: message } = await getClientErrorObject(error);
+ if (impactRequestGeneration.current === generation) {
+ setDatasetPurgeModal({
+ status: 'error',
+ item,
+ message,
+ });
+ }
+ }
+ }, []);
+
+ const closeDatasetPurgeModal = useCallback(() => {
+ impactRequestGeneration.current += 1;
+ setDatasetPurgeModal({ status: 'closed' });
+ }, []);
Review Comment:
Fixed in 25781a051a — the request generation now gates only the modal-state
update; a purge that succeeds after the modal was closed still fires the
success toast and refreshes the list. Covered by a test with a deferred purge
response that fails against the previous code (`a purge that succeeds after the
modal closes still toasts and refetches`).
_Reply generated by Claude (AI) on behalf of @mikebridge._
##########
superset/commands/deletion_retention/purge_cascade.py:
##########
@@ -241,15 +248,22 @@ def cascade_hard_delete(
if session.execute(claim.with_for_update()).scalar_one_or_none()
is None:
raise PurgeRaceLostError
+ if entity_type == "dataset" and confirmed_impact_token is not None:
+ confirmed_impact = collect_dataset_purge_impact(session,
entity_id)
+ if confirmed_impact.impact_token != confirmed_impact_token:
+ raise PurgeImpactChangedError(confirmed_impact)
Review Comment:
This boundary is deliberate and documented, so no change here. The contract
this PR implements is two submit-time rechecks — one when the purge request
arrives and one after the dataset row claim under `SELECT … FOR UPDATE` — not
global serialization of chart writers. A chart inserted after the locked
recheck can only be excluded by locking every chart/dashboard writer against
every purge, which the spec evaluated and rejected (the same finding was raised
and adjudicated in the pre-PR review). Two mitigations bound the window: the
locked recheck compares full fingerprints (not counts), so anything committed
before the claim forces a 409 with refreshed impact; and the non-cascade design
means a post-claim insert is orphaned but never deleted — the same outcome that
datasource reassignment to a live dataset can produce with no purge involved.
_Reply generated by Claude (AI) on behalf of @mikebridge._
--
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]
