mikebridge commented on code in PR #41550:
URL: https://github.com/apache/superset/pull/41550#discussion_r3692118888


##########
superset/views/base.py:
##########
@@ -510,6 +544,8 @@ def cached_common_bootstrap_data(  # pylint: 
disable=unused-argument
     # should not expose API TOKEN to frontend
     frontend_config = {k: _get_frontend_config_value(k) for k in 
FRONTEND_CONF_KEYS}
 
+    frontend_config.update(_soft_delete_conf())

Review Comment:
   Re-raised on the new head — see the disposition on the earlier identical 
thread: the 60-second staleness is shared by every value 
`cached_common_bootstrap_data` carries (feature flags, menu, conf); a 
single-key invalidation hook into the shared cache is not worth the coupling.



##########
superset/commands/deletion_retention/purge_cascade.py:
##########
@@ -239,6 +273,28 @@ def cascade_hard_delete(
             entity_uuid=uuid,
             blocked_reason=str(ex),
         )
+    except IntegrityError as ex:
+        # Not a policy decision: a restrictive FK the cascade did not handle.
+        # Two audiences, two messages. The curated reason goes to the caller
+        # (and from there into a user toast), because raw driver text carries
+        # the failing SQL and bind parameters. The constraint detail goes to
+        # the log at WARNING, because an entity permanently unpurgeable via an
+        # unknown FK is a cascade-coverage bug someone has to be able to
+        # diagnose -- reported at INFO as a policy block, it read as intended
+        # behaviour.
+        logger.warning(
+            "deletion_retention: %s id=%s purge failed on a restrictive "
+            "foreign key the cascade does not handle: %s",
+            entity_type,
+            entity_id,
+            ex,
+        )
+        return CascadeResult(
+            purged=False,
+            entity_type=entity_type,
+            entity_uuid=uuid,
+            blocked_reason="blocked by database references",
+        )

Review Comment:
   Re-raised on the new head — see the disposition on the earlier identical 
thread: `blocked_reason` reaches user-facing toasts, so raw driver/SQL text 
stays out by design; the full original error is preserved at WARNING in the 
server log. A machine-readable discriminator, if ever needed, should be a 
parsed constraint-name field, not `str(ex)`.



##########
superset/views/filters.py:
##########
@@ -292,6 +314,37 @@ def _mark_response_for_deleted_at_augmentation() -> None:
         setattr(g, AUGMENT_RESPONSE_WITH_DELETED_AT, True)
 
 
+class BaseDeletedRecencyFilter(BaseFilter):  # pylint: 
disable=too-few-public-methods
+    """Keep rows archived within the last *value* days, by the server's clock.
+
+    The archive UI's time-range presets used to send an absolute cutoff
+    computed client-side in UTC. ``deleted_at`` is stamped with the server's
+    naive-local ``datetime.now()``, so on any non-UTC deployment those
+    cutoffs were shifted by the server offset -- and because the cutoff was
+    frozen when the page mounted, a long-lived tab drifted further. Taking a
+    day count and resolving it here, on the clock that stamped the column,
+    removes both failure modes and lets the client keep stable, shareable
+    filter values.
+
+    Subclasses set ``arg_name`` (e.g. ``"chart_deleted_recency"``).
+    """
+
+    name = lazy_gettext("Archived within")
+
+    def apply(self, query: Query, value: Any) -> Query:
+        try:
+            days = int(value)
+        except (TypeError, ValueError):
+            # Filter values arrive from the URL; refusing loudly would turn a
+            # mangled query string into a 500. An unfiltered list is the same
+            # answer every other malformed FAB filter value produces.
+            return query
+        if days <= 0:
+            return query
+        cutoff = datetime.now() - timedelta(days=days)
+        return query.filter(self.model.deleted_at > cutoff)

Review Comment:
   Re-raised on the new head — see the disposition on the earlier identical 
thread: the recency filter deliberately does not imply the visibility bypass, 
because the bypass carries the restore-audience scoping enforced by 
`deleted_state`; the two compose, and standalone recency returning nothing is 
the fail-closed direction.



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