sadpandajoe commented on code in PR #45894:
URL: https://github.com/apache/superset/pull/45894#discussion_r4460859869
##########
superset-frontend/src/pages/Chart/index.tsx:
##########
@@ -379,5 +413,17 @@ export default function ExplorePage() {
if (!isLoaded) {
return <Loading />;
}
+ if (authenticationError) {
+ return (
+ <OAuth2RedirectMessage
Review Comment:
This prompt replaces the whole Explore view, but `closable` isn't passed, so
`ErrorAlert` defaults it to `true`. If the user dismisses the alert before
authorizing, the authorization link disappears and nothing is left on the page
(no Explore UI and no retry control) until they reload. Should this pass
`closable={false}`?
##########
superset/semantic_layers/registry.py:
##########
@@ -17,8 +17,264 @@
from __future__ import annotations
-from typing import Any
+from contextlib import contextmanager, nullcontext
+from functools import wraps
+from typing import Any, Iterator
+from superset_core.extensions.types import Manifest
from superset_core.semantic_layers.layer import SemanticLayer
+from superset.extensions.context import (
+ extension_context,
+ get_current_extension_context,
+)
+from superset.semantic_layers.oauth2 import check_for_oauth2
+from superset.utils.core import get_user_id
+
registry: dict[str, type[SemanticLayer[Any, Any]]] = {}
+_extension_manifests: dict[str, Manifest] = {}
+
+
+def register_semantic_layer(
+ semantic_layer_id: str,
+ implementation: type[SemanticLayer[Any, Any]],
+ manifest: Manifest | None = None,
+) -> None:
+ """Register a provider and retain the extension that owns it."""
+ registry[semantic_layer_id] = implementation
+ if manifest is None:
+ _extension_manifests.pop(semantic_layer_id, None)
+ else:
+ _extension_manifests[semantic_layer_id] = manifest
+
+
+def unregister_semantic_layer(semantic_layer_id: str) -> None:
+ """Remove a provider and any retained extension metadata."""
+ registry.pop(semantic_layer_id, None)
+ _extension_manifests.pop(semantic_layer_id, None)
+
+
+@contextmanager
+def semantic_layer_context(
+ semantic_layer_id: str,
+ layer_uuid: str | None = None,
+ view_uuid: str | None = None,
+) -> Iterator[None]:
+ """Run provider code in its owning extension and principal context."""
+ manifest = _extension_manifests.get(semantic_layer_id)
+ current_context = get_current_extension_context()
+ user_id = (
+ current_context.user_id
+ if current_context is not None and current_context.user_id is not None
+ else get_user_id()
+ )
+ context = (
+ extension_context(manifest, user_id=user_id)
+ if manifest is not None
+ else nullcontext()
+ )
+ with check_for_oauth2(layer_uuid, view_uuid), context:
+ yield
+
+
+def _contextual_attribute(
+ semantic_layer_id: str,
+ implementation: Any,
+ name: str,
+ layer_uuid: str | None,
+ view_uuid: str | None,
+) -> Any:
+ """Keep both property access and fallback method calls in provider
context."""
+ with semantic_layer_context(semantic_layer_id, layer_uuid, view_uuid):
+ attribute = getattr(implementation, name)
+ if not callable(attribute):
+ return attribute
+
+ @wraps(attribute)
+ def contextualized(*args: Any, **kwargs: Any) -> Any:
+ with semantic_layer_context(semantic_layer_id, layer_uuid, view_uuid):
+ return attribute(*args, **kwargs)
+
+ return contextualized
+
+
+class ContextualSemanticView:
+ """Forward semantic view operations inside their extension context."""
+
+ def __init__(
+ self,
+ semantic_layer_id: str,
+ implementation: Any,
+ layer_uuid: str | None = None,
+ view_uuid: str | None = None,
+ ):
+ self._semantic_layer_id = semantic_layer_id
+ self._implementation = implementation
+ self._layer_uuid = layer_uuid
+ self._view_uuid = view_uuid
+
+ def __getattr__(self, name: str) -> Any:
+ return _contextual_attribute(
+ self._semantic_layer_id,
+ self._implementation,
+ name,
+ self._layer_uuid,
+ self._view_uuid,
+ )
+
+ def __hash__(self) -> int:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return hash(self._implementation)
+
+ def __eq__(self, other: object) -> bool:
+ if isinstance(other, ContextualSemanticView):
+ other = other._implementation
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation == other
+
+ def uid(self) -> str:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.uid()
+
+ def get_dimensions(self) -> Any:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.get_dimensions()
+
+ def get_metrics(self) -> Any:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.get_metrics()
+
+ def get_values(self, dimension: Any, filters: Any = None) -> Any:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.get_values(dimension, filters)
+
+ def get_table(self, query: Any) -> Any:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.get_table(query)
+
+ def get_row_count(self, query: Any) -> Any:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.get_row_count(query)
+
+ def get_compatible_metrics(
+ self,
+ selected_metrics: Any,
+ selected_dimensions: Any,
+ ) -> Any:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.get_compatible_metrics(
+ selected_metrics,
+ selected_dimensions,
+ )
+
+ def get_compatible_dimensions(
+ self,
+ selected_metrics: Any,
+ selected_dimensions: Any,
+ ) -> Any:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ return self._implementation.get_compatible_dimensions(
+ selected_metrics,
+ selected_dimensions,
+ )
+
+
+class ContextualSemanticLayer:
+ """Forward semantic layer operations inside their extension context."""
+
+ def __init__(
+ self,
+ semantic_layer_id: str,
+ implementation: Any,
+ layer_uuid: str | None = None,
+ view_uuid: str | None = None,
+ ):
+ self._semantic_layer_id = semantic_layer_id
+ self._implementation = implementation
+ self._layer_uuid = layer_uuid
+ self._view_uuid = view_uuid
+
+ def __getattr__(self, name: str) -> Any:
+ return _contextual_attribute(
+ self._semantic_layer_id,
+ self._implementation,
+ name,
+ self._layer_uuid,
+ self._view_uuid,
+ )
+
+ def get_semantic_views(self, runtime_configuration: dict[str, Any]) ->
set[Any]:
+ with semantic_layer_context(
+ self._semantic_layer_id, self._layer_uuid, self._view_uuid
+ ):
+ views =
self._implementation.get_semantic_views(runtime_configuration)
+ return {
+ ContextualSemanticView(
+ self._semantic_layer_id, view, self._layer_uuid,
self._view_uuid
+ )
+ for view in views
+ }
+
+ def get_semantic_view(
+ self,
+ name: str,
+ additional_configuration: dict[str, Any],
+ ) -> ContextualSemanticView:
+ with semantic_layer_context(
Review Comment:
`get_semantic_view` now raises `SemanticLayerOAuth2RedirectError` when the
user's grant is missing or expired, but its only caller,
`CreateSemanticViewCommand.validate`, wraps this call in `except Exception` and
turns it into `view = None`. Since the redirect error is swallowed,
`AddSemanticViewModal` shows the generic "Couldn't validate this semantic view"
failure with no authorization link. If a user's grant is revoked or expires
between listing views and clicking Add, retrying Add repeats the same failure.
Should the create command let the OAuth redirect error propagate (and the bulk
create endpoint return it) so the modal can offer authorization?
##########
superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx:
##########
@@ -485,6 +555,22 @@ export default function AddSemanticViewModal({
refreshGen !== schemaRefreshGenRef.current
)
return;
+ if (
+ await captureAuthenticationError(
+ error,
+ () => {
+ lastDepSnapshotRef.current = '';
+ handleRuntimeFormChange({
Review Comment:
The retry saved when a runtime-schema refresh needs authorization replays
the `data` captured when that refresh started, and nothing clears
`authentication` when the form is edited (only a layer change or hiding the
modal does). If the user opens the authorization link, edits a runtime field,
then finishes authorizing, the retry calls `handleRuntimeFormChange` with the
old `data` and overwrites the newer edits. Should the retry use the current
form state, or discard the pending prompt once the form changes?
--
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]
