sadpandajoe commented on code in PR #44851:
URL: https://github.com/apache/superset/pull/44851#discussion_r4161585686
##########
superset-frontend/src/explore/actions/exploreActions.ts:
##########
@@ -272,6 +284,75 @@ export function syncDatasourceMetadata(datasource:
Dataset) {
return { type: SYNC_DATASOURCE_METADATA, datasource };
}
+export const SYNC_SEMANTIC_METADATA = 'explore/SYNC_SEMANTIC_METADATA';
+/** Rebuild metadata-derived controls without recording a chart edit. */
+export function syncSemanticMetadata(
+ datasource: Dataset,
+ formData: QueryFormData,
+): {
+ type: typeof SYNC_SEMANTIC_METADATA;
+ datasource: Dataset;
+ formData: QueryFormData;
+} {
+ return { type: SYNC_SEMANTIC_METADATA, datasource, formData };
+}
+
+/** Refresh the active view's metadata without saving or running the chart. */
+export function refreshSemanticMetadata(
+ viewId: number,
+ sessionIsCurrent: () => boolean,
+) {
+ return async (
+ dispatch: Dispatch,
+ getState: () => Pick<ExplorePageState, 'explore'>,
+ ) => {
+ const isCurrent = () => {
+ const { datasource } = getState().explore;
+ return (
+ sessionIsCurrent() &&
+ Number(datasource.id) === viewId &&
+ String(datasource.type) === 'semantic_view'
+ );
+ };
+ if (!isCurrent()) return;
+ // A pre-sync compatibility response cannot replace a post-sync answer.
+ compatibilityRequestSeq += 1;
+ const { json } = await SupersetClient.get({
+ endpoint:
`/fetch_datasource_metadata?datasourceKey=${viewId}__semantic_view`,
+ });
+ if (!isCurrent()) return;
+ const formData = getFormDataFromControls(getState().explore.controls);
+ // Rebuild the controls against fresh fields using their existing values
and
+ // normal removed-member validation, without rewriting form_data or
querying.
+ dispatch(syncSemanticMetadata(json as Dataset, formData));
+ const selectedMetrics = [
+ ...new Set(
+ [...ensureIsArray(formData.metrics), formData.metric].filter(
Review Comment:
After syncing a Big Number chart, compatibility is constrained by its
singleton metric, but Explore’s ongoing effect watches only metrics, not
metric. Clearing metric A therefore leaves the picker disabling metric B
against A’s stale answer when A and B cannot combine; could the effect share
this selection extraction and rerun on singleton changes?
##########
superset/coordination/deadline_backend.py:
##########
@@ -0,0 +1,228 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Private, cancellation-bounded Redis operations for synchronous metadata
callers.
+
+Each command owns its async client and loop. Cancelling the command disconnects
+its socket; shared coordinator pools and their retry/timeout policy are
untouched.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import math
+import time
+from contextlib import AsyncExitStack
+from typing import Any
+
+from redis.asyncio import Redis
+from redis.asyncio.retry import Retry
+from redis.asyncio.sentinel import Sentinel
+from redis.backoff import NoBackoff
+from redis.exceptions import RedisError, TimeoutError as RedisTimeoutError
+from superset_core.semantic_layers.metadata import (
+ MetadataRefreshError,
+ remaining_budget,
+)
+
+from superset.coordination.cache_backend import _COMPARE_AND_DELETE_LUA
+
+_COMPARE_AND_PUBLISH_LUA: str = """
+if redis.call('get', KEYS[1]) ~= ARGV[1] then
+ return 0
+end
+redis.call('psetex', KEYS[2], ARGV[3], ARGV[2])
+redis.call('del', KEYS[1])
+return 1
+"""
+
+_GET_WITH_TTL_LUA: str = """
+return {redis.call('get', KEYS[1]), redis.call('pttl', KEYS[1])}
+"""
+
+_GET_OR_CREATE_LUA: str = """
+local value = redis.call('get', KEYS[1])
+if value then
+ return value
+end
+redis.call('set', KEYS[1], ARGV[1], 'EX', ARGV[2])
+return ARGV[1]
+"""
+
+
+class DeadlineRedisBackend:
+ """Use the coordinator configuration without sharing mutable
connections."""
+
+ def __init__(self, config: dict[str, Any], *, deadline: float) -> None:
+ if not math.isfinite(deadline) or config.get("CACHE_TYPE") not in {
+ "RedisCache",
+ "RedisSentinelCache",
+ }:
+ raise ValueError("Unsupported metadata coordination configuration")
+ self._config: dict[str, Any] = dict(config)
+ self._deadline: float = deadline
+
+ def with_deadline(self, deadline: float) -> DeadlineRedisBackend:
+ """Create a private call budget without extending the operation
ceiling."""
+ if not math.isfinite(deadline):
+ raise ValueError("Metadata deadline must be finite")
+ return DeadlineRedisBackend(
+ self._config, deadline=min(self._deadline, deadline)
+ )
+
+ def _remaining(self) -> float:
+ """Keep the transport's Redis error boundary while sharing SDK
validation."""
+ try:
+ return remaining_budget(self._deadline, now=time.monotonic())
+ except MetadataRefreshError:
+ raise RedisTimeoutError("Metadata deadline invalid or expired")
from None
+
+ async def _command(self, *args: str | int) -> Any:
+ remaining: float = self._remaining()
+ options: dict[str, Any] = {
+ "db": self._config.get("CACHE_REDIS_DB", 0),
+ "username": self._config.get("CACHE_REDIS_USER"),
+ "password": self._config.get("CACHE_REDIS_PASSWORD"),
+ "socket_timeout": remaining,
+ "socket_connect_timeout": remaining,
+ "retry": Retry(NoBackoff(), 0),
+ "protocol": 2,
+ }
+ if self._config.get("CACHE_REDIS_SSL", False):
+ options.update(
+ {
+ "ssl": True,
+ "ssl_certfile":
self._config.get("CACHE_REDIS_SSL_CERTFILE"),
+ "ssl_keyfile": self._config.get("CACHE_REDIS_SSL_KEYFILE"),
+ "ssl_ca_certs":
self._config.get("CACHE_REDIS_SSL_CA_CERTS"),
+ "ssl_cert_reqs": self._config.get(
+ "CACHE_REDIS_SSL_CERT_REQS", "required"
+ ),
+ }
+ )
+ # One cancellation deadline covers DNS, Sentinel discovery,
authentication,
+ # response parsing (including trickled responses) and connection
cleanup.
+ stack: AsyncExitStack
+ async with asyncio.timeout(remaining), AsyncExitStack() as stack:
+ client: Redis
+ if self._config["CACHE_TYPE"] == "RedisSentinelCache":
+ sentinel: Sentinel = Sentinel(
+ self._config.get("CACHE_REDIS_SENTINELS", [("127.0.0.1",
26379)]),
+ sentinel_kwargs={
+ "password":
self._config.get("CACHE_REDIS_SENTINEL_PASSWORD"),
+ "socket_timeout": remaining,
+ "socket_connect_timeout": remaining,
+ "retry": Retry(NoBackoff(), 0),
+ "protocol": 2,
+ },
+ **options,
+ )
+ sentinel_client: Redis
+ for sentinel_client in sentinel.sentinels:
+ stack.push_async_callback(sentinel_client.aclose)
+ client = sentinel.master_for(
+ self._config.get("CACHE_REDIS_SENTINEL_MASTER", "mymaster")
+ )
+ else:
+ client = Redis(
+ host=self._config.get("CACHE_REDIS_HOST", "localhost"),
+ port=self._config.get("CACHE_REDIS_PORT", 6379),
+ **options,
+ )
+ await stack.enter_async_context(client)
+ return await client.execute_command(*args)
+
+ def execute(self, *args: str | int) -> Any:
+ """Run one command with no automatic retry or detached Redis
commands."""
+ self._remaining()
+ try:
+ asyncio.get_running_loop()
+ except RuntimeError:
+ pass
Review Comment:
Under the documented gevent Gunicorn setup, concurrent synchronous requests
share the native thread: while one request’s Redis loop yields, the next sees
that running loop and raises “Metadata backend requires a synchronous caller,”
producing unavailable responses even with healthy Redis. Could this bridge
isolate its loop from other request greenlets, with concurrent-greenlet
coverage?
##########
superset-frontend/src/explore/actions/exploreActions.ts:
##########
@@ -223,8 +233,8 @@ export function fetchCompatibility(
selected_dimensions: selectedDimensions,
},
});
- if (requestSeq !== compatibilityRequestSeq) {
- return;
+ if (requestSeq !== compatibilityRequestSeq || !isCurrent()) {
Review Comment:
Closing the editor after the compatibility POST starts makes isCurrent()
false, so the response is discarded while Explore stays in loading; with
unchanged selections its compatibility effect never reruns, leaving choices
unfiltered without a failure warning. Could this settle the active request
independently of the modal lifetime, with a deferred-POST regression that
closes after loading starts?
--
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]
