mikebridge commented on code in PR #44851:
URL: https://github.com/apache/superset/pull/44851#discussion_r4213732085
##########
superset-frontend/src/features/semanticViews/SemanticViewEditModal.tsx:
##########
@@ -188,15 +218,102 @@ export default function SemanticViewEditModal({
});
addSuccessToast?.(t('Semantic view updated'));
onSave();
Review Comment:
Confirmed. UI commit
[6d61811a40](https://github.com/apache/superset/commit/6d61811a405ed10417dde4ebdb159b1739e9e1b5)
guards onSave with the same operation generation as modal closure, so an old
PUT can still report its outcome without replaying a stale Explore datasource
callback. The regression starts Save, closes/reopens, completes a newer sync,
then resolves Save and verifies no stale callback or modal close and the
refreshed structure remains. One side effect: a Save that finishes after the
editor has closed no longer refreshes the dataset list, so the list may need a
manual reload in that case.
##########
superset/common/query_context_processor.py:
##########
@@ -428,28 +448,100 @@ def query_cache_key(self, query_obj: QueryObject,
**kwargs: Any) -> str | None:
"""
Returns a QueryObject cache key for objects in self.queries
"""
+ return self._query_cache_key(query_obj, **kwargs)[0]
+
+ def _query_cache_key(
+ self,
+ query_obj: QueryObject,
+ *,
+ parent_cache_context: dict[str, Any] | None = None,
+ **kwargs: Any,
+ ) -> tuple[str | None, bool]:
+ """Keep annotation cacheability alongside the opaque hashed result
key."""
datasource: Explorable = self._qc_datasource
- # Reject unenforceable restrictions before provider identity or cache
reads.
- rls: list[str] = security_manager.get_rls_cache_key(datasource)
- extra_cache_keys = datasource.get_extra_cache_keys(query_obj.to_dict())
+ if parent_cache_context is None:
+ parent_cache_context = {}
+ if not parent_cache_context:
+ # Capture parent/Jinja inputs once; annotation discovery may re-key
+ # the result after execution but must not evaluate new parent
inputs.
+ rls: list[str] = security_manager.get_rls_cache_key(datasource)
+ parent_cache_context.update(
+ datasource=datasource.uid,
+
extra_cache_keys=datasource.get_extra_cache_keys(query_obj.to_dict()),
+ rls=rls,
+ changed_on=datasource.changed_on,
+ )
+ cacheable: bool = True
# Annotation data is cached on the same entry as the dataframe, so the
# key must also bind the annotation sources' security context.
if query_obj and query_obj.annotation_layers:
- kwargs["annotation_context"] =
self._annotation_cache_context(query_obj)
+ annotation_context: dict[str, Any] =
self._annotation_cache_context(
+ query_obj
+ )
+ kwargs["annotation_context"] = annotation_context
+ source_metadata: dict[str, str] = annotation_context.get(
+ "source_metadata", {}
+ )
+ cacheable = not any(
+ token.startswith("uncaptured:") for token in
source_metadata.values()
+ )
- cache_key = (
+ cache_key: str | None = (
query_obj.cache_key(
- datasource=datasource.uid,
- extra_cache_keys=extra_cache_keys,
- rls=rls,
- changed_on=datasource.changed_on,
+ **parent_cache_context,
**kwargs,
)
if query_obj
else None
)
- return cache_key
+ if cache_key is not None:
+ capture_result_identity(self._query_context, query_obj, cache_key)
+ return cache_key, cacheable
+
+ def _capture_annotation_metadata(self, query_obj: QueryObject) -> None:
+ """Capture authorized annotation views on a miss before a slow parent
query."""
+ if not query_obj.annotation_layers:
+ return
+ from superset_core.semantic_layers.metadata import MetadataRefreshError
+
+ from superset.semantic_layers.metadata_binding import (
+ metadata_refresh_enabled,
+ participates,
+ )
+ from superset.semantic_layers.models import SemanticView
+
+ if not metadata_refresh_enabled():
+ return
+ layer: dict[str, Any]
+ for layer in query_obj.annotation_layers:
+ if (
+ layer.get("sourceType")
+ not in ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ ):
+ continue
+ value: int | str | None = layer.get("value")
+ chart: Slice | None = (
+ ChartDAO.find_by_id(value) if value is not None else None
+ )
+ if chart is None:
+ continue
+ try:
+ context: QueryContext | None = chart.get_query_context()
+ if context is None:
+ # The annotation executor reports its missing-context
error.
+ continue
+ source: Explorable = context.datasource
+ if not isinstance(source, SemanticView) or not participates(
+ source.semantic_layer
+ ):
+ continue
+ context.raise_for_access()
+ # Reuse the annotation command's canonical query authority.
+ # Later execution retains this view without renewing its
budget.
+ _captured: str | None = source.metadata_cache_token
+ except (SupersetException, MetadataRefreshError) as ex:
+ raise QueryObjectValidationError(error_msg_from_exception(ex))
from ex
Review Comment:
Confirmed and repaired in owning store commit
[34f90b5244](https://github.com/apache/superset/commit/34f90b5244a22e8b7e2d7be62e6953314562c9f0),
merged into local UI
[6d61811a40](https://github.com/apache/superset/commit/6d61811a405ed10417dde4ebdb159b1739e9e1b5).
Typed metadata errors escape annotation validation conversion and reach the
API's existing GET/POST mapper. Processor-plus-adapter regressions
demonstrate503/504/502 instead of400, while existing route mapping tests pass;
no new live annotation HTTP run is claimed.
##########
superset-frontend/src/features/semanticViews/SemanticViewEditModal.tsx:
##########
@@ -188,15 +218,102 @@ export default function SemanticViewEditModal({
});
addSuccessToast?.(t('Semantic view updated'));
onSave();
- onHide();
+ if (isCurrent()) handleHide();
} catch (error) {
const clientError = await getClientErrorObject(error);
addDangerToast?.(
clientError.error ||
t('An error occurred while saving the semantic view'),
);
} finally {
- setSaving(false);
+ if (isCurrent()) {
+ busy.current = false;
+ setSaving(false);
+ }
+ }
+ };
+
+ const reloadFields = async (
+ viewId: number,
+ changed: boolean | null,
+ isCurrent: () => boolean,
+ ) => {
+ setSyncState({ status: 'reloading', changed });
+ try {
+ const { json } = await SupersetClient.get({
+ endpoint: `/api/v1/semantic_view/${viewId}/structure`,
+ });
+ if (!isCurrent()) return;
+ setStructure(json.result);
+ await onMetadataSync?.(isCurrent);
+ if (!isCurrent()) return;
+ if (structure?.uuid) dispatch(confirmReload(structure.uuid));
+ setSyncState({ status: 'done', changed });
+ } catch {
+ if (isCurrent()) setSyncState({ status: 'reload-error', changed });
+ }
+ };
+
+ const handleSync = async () => {
+ if (
+ !semanticView ||
+ !structure?.uuid ||
+ structure.can_refresh_metadata !== true ||
+ busy.current ||
+ reloadOnly
+ )
+ return;
+ busy.current = true;
+ generation.current += 1;
+ const requestGeneration = generation.current;
+ const isCurrent = () => requestGeneration === generation.current;
+ setSyncState({ status: 'syncing' });
+ try {
+ const { json } = await SupersetClient.post({
+ endpoint: `/api/v1/semantic_view/${structure.uuid}/refresh_metadata/`,
+ jsonPayload: {},
+ });
+ if (!isCurrent()) return;
+ await reloadFields(
+ semanticView.id,
+ json.result.status === 'changed',
+ isCurrent,
+ );
+ } catch (error) {
+ if (!isCurrent()) return;
+ const { message, reloadRequired } = await metadataSyncError(error);
+ if (!isCurrent()) return;
+ if (reloadRequired)
+ dispatch(markUnconfirmed({ uuid: structure.uuid, message }));
Review Comment:
Confirmed. UI commit
[6d61811a40](https://github.com/apache/superset/commit/6d61811a405ed10417dde4ebdb159b1739e9e1b5)
records an indeterminate outcome against its original view UUID before
discarding stale component updates. Regressions cover rejection while closed,
after reopening, and after a newer sync has completed; each retains the warning
and requires Reload fields before another POST. A later settled success/error
cannot hide that persistent warning, and existing reload-failure recovery stays
intact.
--
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]
