codeant-ai-for-open-source[bot] commented on code in PR #38354:
URL: https://github.com/apache/superset/pull/38354#discussion_r2876674964
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -81,33 +108,34 @@ export const useResultsPane = ({
setForceQuery?.(false);
}
setIsLoading(false);
+ return;
}
- if (isRequest && !cache.has(queryFormData)) {
- setIsLoading(true);
- getChartDataRequest({
- formData: queryFormData,
- force: queryForce,
- resultFormat: 'json',
- resultType: 'results',
- ownState,
+
+ // Fallback: fetch from API (legacy charts without queriesResponse)
+ setIsLoading(true);
+ getChartDataRequest({
+ formData: queryFormData,
+ force: queryForce,
+ resultFormat: 'json',
+ resultType: 'results',
+ ownState,
+ })
+ .then(({ json }) => {
+ setResultResp(ensureIsArray(json.result) as QueryResultInterface[]);
+ setResponseError('');
+ cache.set(queryFormData, json.result);
+ if (queryForce) {
+ setForceQuery?.(false);
+ }
})
- .then(({ json }) => {
- setResultResp(ensureIsArray(json.result) as QueryResultInterface[]);
- setResponseError('');
- cache.set(queryFormData, json.result);
- if (queryForce) {
- setForceQuery?.(false);
- }
- })
- .catch(response => {
- getClientErrorObject(response).then(({ error, message }) => {
- setResponseError(error || message || t('Sorry, an error
occurred'));
- });
- })
- .finally(() => {
- setIsLoading(false);
+ .catch(response => {
+ getClientErrorObject(response).then(({ error, message }) => {
+ setResponseError(error || message || t('Sorry, an error occurred'));
});
- }
+ })
+ .finally(() => {
+ setIsLoading(false);
+ });
}, [queryFormData, isRequest]);
Review Comment:
**Suggestion:** The effect that populates the results table does not list
the reused chart data (`queriesResponse`) in its dependency array, so when new
chart data arrives with the same `queryFormData` and unchanged `isRequest`, the
hook will not re-run and the table will show stale results; adding
`queriesResponse` as a dependency ensures the hook reacts whenever the chart
data changes. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Results pane may ignore updated queriesResponse from Redux.
- ⚠️ Extra chart results API calls instead of reusing data.
```
</details>
```suggestion
}, [queryFormData, isRequest, queriesResponse]);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render any component that calls `useResultsPane` (hook defined in
`superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:49-147`)
with some `queryFormData` and `isRequest=true`, and with `queriesResponse`
initially empty
or undefined.
2. On that initial render, the `useEffect` at `useResultsPane.tsx:74-139`
runs because
`queryFormData`/`isRequest` changed; the `queriesResponse?.length` check at
lines 82-90
fails, the cache miss path at lines 101-111 runs, and `getChartDataRequest`
at lines
115-123 fetches results and populates `resultResp`.
3. Later, update the parent component so that it passes a new
`queriesResponse` array
(e.g., Redux chart data for the same `queryFormData`) while keeping
`queryFormData` and
`isRequest` unchanged; React re-renders the parent and `useResultsPane`, but
the effect at
74-139 does not re-run because its dependency array at line 139 is
`[queryFormData,
isRequest]` and does not include `queriesResponse`.
4. Observe that the branch intended to reuse Redux data (`if
(queriesResponse?.length &&
'colnames' in queriesResponse[0])` at lines 82-98) is never executed for the
new
`queriesResponse` value and `resultResp` continues to reflect only the data
from the
earlier `getChartDataRequest` call, meaning the optimization to reuse chart
data is
effectively bypassed whenever `queriesResponse` changes independently of
`queryFormData`/`isRequest`.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
**Line:** 139:139
**Comment:**
*Logic Error: The effect that populates the results table does not list
the reused chart data (`queriesResponse`) in its dependency array, so when new
chart data arrives with the same `queryFormData` and unchanged `isRequest`, the
hook will not re-run and the table will show stale results; adding
`queriesResponse` as a dependency ensures the hook reacts whenever the chart
data changes.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=d5f255a429a4fedb82712c6bdf0cf1300157f585d03042ee1358ddec9bc4347d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=d5f255a429a4fedb82712c6bdf0cf1300157f585d03042ee1358ddec9bc4347d&reaction=dislike'>👎</a>
##########
superset/charts/data/api.py:
##########
@@ -353,6 +354,70 @@ def data_from_cache(self, cache_key: str) -> Response:
return self._get_data_response(command, True)
+ @expose("/data/stop", methods=("POST",))
+ @protect()
+ @statsd_metrics
+ def stop(self) -> Response:
+ """Stop a running chart query by client_id.
+ ---
+ post:
+ summary: Stop a running chart query
+ description: >-
+ Cancels a running chart query on the database engine using a
+ client-generated ID that was sent with the original data request.
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ client_id:
+ type: string
+ responses:
+ 200:
+ description: Query cancelled
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ """
+ if not request.is_json or not request.json:
+ return self.response_400(message=_("Request is not JSON"))
+
+ client_id = request.json.get("client_id")
+ if not client_id:
+ return self.response_400(message=_("client_id is required"))
+
+ cancel_info = cache_manager.inflight_query_cache.get(client_id)
+ if not cancel_info:
+ return self.response_404()
+
+ database = db.session.query(Database).get(cancel_info["database_id"])
Review Comment:
**Suggestion:** The stop endpoint cancels queries based solely on a
client-provided `client_id` without verifying that the current user owns that
inflight query, allowing any authenticated user who knows a `client_id` to
cancel another user's query; you should compare the stored user identifier in
`cancel_info` with the current user ID and return 403 when they don't match.
[security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Authenticated users can cancel other users' running queries.
- ⚠️ Enables targeted denial-of-service against specific users' charts.
- ⚠️ Breaks expectation of per-user isolation for query control.
```
</details>
```suggestion
cancel_info = cache_manager.inflight_query_cache.get(client_id)
if not cancel_info:
return self.response_404()
if cancel_info.get("user_id") != get_user_id():
return self.response_403()
database = db.session.query(Database).get(cancel_info["database_id"])
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. User A runs a chart using `ChartDataRestApi.data` in
`superset/charts/data/api.py:120-190` with async queries enabled, which
triggers async
query handling that stores an inflight record in
`cache_manager.inflight_query_cache`
keyed by a client-generated `client_id` (used later by `stop()`).
2. User B, a different authenticated Superset user, obtains User A's
`client_id` for that
inflight query (for example, by observing network traffic or logs in a
shared environment
where client IDs are visible).
3. User B sends an authenticated POST request to the stop endpoint
`/data/stop` handled by
`ChartDataRestApi.stop` in `superset/charts/data/api.py:357-419` with JSON
body
`{"client_id": "<User A's client_id>"}`.
4. In `stop()`, lines 394-398 fetch `cancel_info` from
`cache_manager.inflight_query_cache` and immediately look up the `Database`
using
`cancel_info["database_id"]` without checking that `cancel_info` belongs to
`get_user_id()`, so `database.db_engine_spec.cancel_query(...)` is executed
against User
A's query, allowing User B to cancel another user's running query.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/data/api.py
**Line:** 394:398
**Comment:**
*Security: The stop endpoint cancels queries based solely on a
client-provided `client_id` without verifying that the current user owns that
inflight query, allowing any authenticated user who knows a `client_id` to
cancel another user's query; you should compare the stored user identifier in
`cancel_info` with the current user ID and return 403 when they don't match.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=8100503e13b21f8201e6d07bbdd69bbfcf6457408f18d97edfe6fc3c5617441a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=8100503e13b21f8201e6d07bbdd69bbfcf6457408f18d97edfe6fc3c5617441a&reaction=dislike'>👎</a>
##########
superset/models/core.py:
##########
@@ -692,34 +797,60 @@ def _execute_sql_with_mutation_and_logging(
log_query = app.config["QUERY_LOGGER"]
- def _log_query(sql_: str) -> None:
- if log_query:
- log_query(
- engine_url,
- sql_,
- schema,
- __name__,
- security_manager,
- )
+ inflight_cache = None
+ if client_id and not self.db_engine_spec.has_implicit_cancel():
+ inflight_cache = cache_manager.inflight_query_cache
with self.get_raw_connection(catalog=catalog, schema=schema) as conn:
cursor = conn.cursor()
rows = None
description = None
+ # Capture cancel_query_id before execute (Postgres, MySQL, etc.)
+ cancel_info_stored = False
+ if inflight_cache is not None:
+ assert client_id is not None
+ cancel_info_stored = self._store_cancel_info(
+ cursor, client_id, catalog, schema, inflight_cache,
+ )
+
for i, statement in enumerate(script.statements):
sql_ = self.mutate_sql_based_on_config(
statement.format(),
is_split=True,
)
- _log_query(sql_)
+ if log_query:
+ log_query(
+ engine_url, sql_, schema, __name__, security_manager,
+ )
- with event_logger.log_context(
- action="execute_sql",
- database=self,
- object_ref=__name__,
- ):
- self.db_engine_spec.execute(cursor, sql_, self)
+ # For engines where cancel_query_id is only available DURING
+ # execute (e.g. Trino), run execute in a thread and poll for
+ # the query_id to appear on the cursor.
+ need_threaded = (
+ inflight_cache is not None and not cancel_info_stored
+ )
+
+ if need_threaded:
+ assert client_id is not None
+ self._execute_with_cancel_capture(
+ cursor, sql_, client_id, catalog, schema,
+ inflight_cache,
+ )
+ else:
+ try:
+ with event_logger.log_context(
+ action="execute_sql",
+ database=self,
+ object_ref=__name__,
+ ):
+ self.db_engine_spec.execute(cursor, sql_, self)
+ except Exception as ex:
+ if cancel_info_stored:
+ raise SupersetCancelQueryException(
+ "Query was cancelled"
+ ) from ex
+ raise
Review Comment:
**Suggestion:** In the synchronous execution path, any exception raised
while `cancel_info_stored` is true is blindly converted into a
`SupersetCancelQueryException`, which will incorrectly report normal database
errors (syntax errors, connection issues, etc.) as "Query was cancelled" and
hide the real failure cause; removing this special-case conversion restores
correct error reporting. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Callers of Database.get_df see misleading cancellation errors.
- ⚠️ Actual database issues hidden behind generic cancellation exception.
- ⚠️ Troubleshooting query failures in Explore becomes significantly harder.
```
</details>
```suggestion
else:
with event_logger.log_context(
action="execute_sql",
database=self,
object_ref=__name__,
):
self.db_engine_spec.execute(cursor, sql_, self)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use a database backend where `get_cancel_query_id()` returns a non-None
ID before
execute (commented as "Postgres, MySQL, etc." at
`superset/models/core.py:809-815`) and
where `has_implicit_cancel()` returns False in the engine spec.
2. From Python (e.g. a unit test), call `Database.get_df()` at
`superset/models/core.py:897` with a non-None `client_id`, for example:
`db.get_df("SELECT * FROM bad_syntax", client_id="test-client")`, so that
`_execute_sql_with_mutation_and_logging()` at
`superset/models/core.py:790` is invoked
with that `client_id`.
3. Inside `_execute_sql_with_mutation_and_logging()`, observe that
`inflight_cache` is set
(lines `800-802`) and `_store_cancel_info()` at
`superset/models/core.py:750-772` is
called, which obtains a `cancel_query_id` and sets `cancel_info_stored =
True` for this
cursor before the invalid SQL is executed.
4. When the invalid SQL is executed in the non-threaded branch at
`superset/models/core.py:840-847`, `self.db_engine_spec.execute(cursor,
sql_, self)`
raises a normal database exception (e.g. syntax error). The `except` block at
`superset/models/core.py:848-853` sees `cancel_info_stored` is True and
wraps this normal
error in `SupersetCancelQueryException("Query was cancelled")`, causing
callers of
`get_df()` to receive a misleading "Query was cancelled" error instead of
the real
database failure.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/core.py
**Line:** 840:853
**Comment:**
*Logic Error: In the synchronous execution path, any exception raised
while `cancel_info_stored` is true is blindly converted into a
`SupersetCancelQueryException`, which will incorrectly report normal database
errors (syntax errors, connection issues, etc.) as "Query was cancelled" and
hide the real failure cause; removing this special-case conversion restores
correct error reporting.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=fadec928b54b59c741fed4d9ad5b7d32c0bd858c64d62622d1ed9d8d671c8b58&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=fadec928b54b59c741fed4d9ad5b7d32c0bd858c64d62622d1ed9d8d671c8b58&reaction=dislike'>👎</a>
##########
superset/charts/data/api.py:
##########
@@ -353,6 +354,70 @@ def data_from_cache(self, cache_key: str) -> Response:
return self._get_data_response(command, True)
+ @expose("/data/stop", methods=("POST",))
+ @protect()
+ @statsd_metrics
+ def stop(self) -> Response:
+ """Stop a running chart query by client_id.
+ ---
+ post:
+ summary: Stop a running chart query
+ description: >-
+ Cancels a running chart query on the database engine using a
+ client-generated ID that was sent with the original data request.
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ client_id:
+ type: string
+ responses:
+ 200:
+ description: Query cancelled
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ """
+ if not request.is_json or not request.json:
+ return self.response_400(message=_("Request is not JSON"))
+
+ client_id = request.json.get("client_id")
+ if not client_id:
+ return self.response_400(message=_("client_id is required"))
+
+ cancel_info = cache_manager.inflight_query_cache.get(client_id)
+ if not cancel_info:
+ return self.response_404()
+
+ database = db.session.query(Database).get(cancel_info["database_id"])
+ if not database:
+ return self.response_404()
+
+ if database.db_engine_spec.has_implicit_cancel():
+ cache_manager.inflight_query_cache.delete(client_id)
+ return self.response(200, message="Query cancelled (implicit)")
+
+ from contextlib import closing
+
+ with database.get_sqla_engine(
+ catalog=cancel_info.get("catalog"),
+ schema=cancel_info.get("schema"),
+ ) as engine:
+ with closing(engine.raw_connection()) as conn:
+ with closing(conn.cursor()) as cursor:
+ database.db_engine_spec.cancel_query(
+ cursor, None, cancel_info["cancel_query_id"]
+ )
Review Comment:
**Suggestion:** The `database.get_sqla_engine` return value is being used as
a context manager, but it typically returns a SQLAlchemy engine which does not
implement `__enter__/__exit__`, so `with database.get_sqla_engine(...) as
engine:` will raise an AttributeError at runtime when this endpoint is hit;
instead, get the engine normally and only wrap the raw connection and cursor in
context managers. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ /data/stop crashes, queries cannot be cancelled.
- ⚠️ Long-running chart queries keep consuming database resources.
- ⚠️ Users see 500 error when pressing cancel query.
```
</details>
```suggestion
from contextlib import closing
engine = database.get_sqla_engine(
catalog=cancel_info.get("catalog"),
schema=cancel_info.get("schema"),
)
with closing(engine.raw_connection()) as conn:
with closing(conn.cursor()) as cursor:
database.db_engine_spec.cancel_query(
cursor, None, cancel_info["cancel_query_id"]
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start Superset with this PR's code, ensuring charts are accessible
(ChartDataRestApi in
`superset/charts/data/api.py:72` is registered by FAB).
2. Execute a chart query using the `/data` endpoint (`ChartDataRestApi.data`
in
`superset/charts/data/api.py:120-190`) with async queries enabled so that
`cache_manager.inflight_query_cache` stores an entry containing
`database_id` and
`cancel_query_id`.
3. While the query is still running, send an authenticated POST request to
the stop
endpoint `/data/stop` handled by `ChartDataRestApi.stop` in
`superset/charts/data/api.py:357-419` with JSON body `{"client_id": "<the
running query's
client_id>"}`.
4. When `stop()` executes line 408 `with database.get_sqla_engine(...) as
engine:`,
`Database.get_sqla_engine` (from `superset.models.core.Database`) returns a
SQLAlchemy
Engine object which does not implement `__enter__/__exit__`, causing Python
to raise
`AttributeError: __enter__` and the endpoint to return HTTP 500 instead of
cancelling the
query.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/data/api.py
**Line:** 408:416
**Comment:**
*Logic Error: The `database.get_sqla_engine` return value is being used
as a context manager, but it typically returns a SQLAlchemy engine which does
not implement `__enter__/__exit__`, so `with database.get_sqla_engine(...) as
engine:` will raise an AttributeError at runtime when this endpoint is hit;
instead, get the engine normally and only wrap the raw connection and cursor in
context managers.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=8beb68f997c3268dc5971cdef30f96efd4de46ffc4105b7a48c9d34b716402e8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=8beb68f997c3268dc5971cdef30f96efd4de46ffc4105b7a48c9d34b716402e8&reaction=dislike'>👎</a>
##########
docker/docker-bootstrap.sh:
##########
@@ -80,7 +80,7 @@ case "${1}" in
;;
app)
echo "Starting web app (using development server)..."
- flask run -p $PORT --reload --debugger --without-threads --host=0.0.0.0
--exclude-patterns "*/node_modules/*:*/.venv/*:*/build/*:*/__pycache__/*"
+ flask run -p $PORT --reload --debugger --with-threads --host=0.0.0.0
--exclude-patterns "*/node_modules/*:*/.venv/*:*/build/*:*/__pycache__/*"
Review Comment:
**Suggestion:** The added `--exclude-patterns` option to `flask run` is not
a standard Flask CLI flag and will cause the command to fail with "no such
option: --exclude-patterns", preventing the web app from starting in dev
environments. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dev web server fails when starting with `app` mode.
- ❌ Docker-based local Superset environment cannot be brought up.
- ⚠️ Developers blocked from testing UI in containerized dev.
```
</details>
```suggestion
flask run -p $PORT --reload --debugger --with-threads --host=0.0.0.0
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. From the repository root, ensure the script `docker/docker-bootstrap.sh`
is executable
(`chmod +x docker/docker-bootstrap.sh` if needed).
2. Start the dev web app path by running `bash docker/docker-bootstrap.sh
app`, which
passes `app` as `$1` into the case statement at
`docker/docker-bootstrap.sh:70`.
3. The `app)` branch at `docker/docker-bootstrap.sh:82-84` executes the line
at
`docker/docker-bootstrap.sh:83`: `flask run -p $PORT --reload --debugger
--with-threads
--host=0.0.0.0 --exclude-patterns
"*/node_modules/*:*/.venv/*:*/build/*:*/__pycache__/*"`.
4. The Flask CLI (standard `flask` entry point installed with Superset) does
not define a
`--exclude-patterns` option, so it exits with an error `Error: no such
option:
--exclude-patterns`, and the web app never starts.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docker/docker-bootstrap.sh
**Line:** 83:83
**Comment:**
*Logic Error: The added `--exclude-patterns` option to `flask run` is
not a standard Flask CLI flag and will cause the command to fail with "no such
option: --exclude-patterns", preventing the web app from starting in dev
environments.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=72f1c66905da8f79a1fdba58db126712ab3e21a7087c7fbfab898de67874450c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=72f1c66905da8f79a1fdba58db126712ab3e21a7087c7fbfab898de67874450c&reaction=dislike'>👎</a>
##########
superset/utils/cache_manager.py:
##########
@@ -235,6 +236,11 @@ def init_app(self, app: Flask) -> None:
"EXPLORE_FORM_DATA_CACHE_CONFIG",
required=True,
)
+ self._init_cache(
+ app,
+ self._inflight_query_cache,
+ "INFLIGHT_QUERY_STATE_CACHE_CONFIG",
+ )
Review Comment:
**Suggestion:** The new inflight query cache initialization unconditionally
indexes `app.config["INFLIGHT_QUERY_STATE_CACHE_CONFIG"]` via `_init_cache`, so
if this optional config key is not defined (which will be true on existing
deployments that haven't added it yet), application startup will crash with a
`KeyError` even though the cache is not marked as required. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Superset Flask app fails to start with default config.
- ❌ All HTTP endpoints unavailable due to startup crash.
- ⚠️ New inflight query state feature unusable without manual config.
```
</details>
```suggestion
if "INFLIGHT_QUERY_STATE_CACHE_CONFIG" in app.config:
self._init_cache(
app,
self._inflight_query_cache,
"INFLIGHT_QUERY_STATE_CACHE_CONFIG",
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use a Superset deployment that has not defined
`INFLIGHT_QUERY_STATE_CACHE_CONFIG` in
its configuration (the base config in `superset/config.py` in this repo
snapshot does not
contain this key).
2. Start the Superset web application, which constructs the Flask app in
`superset/app.py:create_app()` and calls `cache_manager.init_app(app)` (see
`superset/app.py` where `from superset.extensions import cache_manager` is
used and
`cache_manager.init_app(app)` is invoked during startup).
3. During `create_app()`, execution enters `CacheManager.init_app()` at
`superset/utils/cache_manager.py:226-244`, reaching the new block:
`self._init_cache(app, self._inflight_query_cache,
"INFLIGHT_QUERY_STATE_CACHE_CONFIG")` (lines 239–243 in the diff).
4. `_init_cache()` at `superset/utils/cache_manager.py:200-224` executes
`cache_config =
app.config[cache_config_key]` with
`cache_config_key="INFLIGHT_QUERY_STATE_CACHE_CONFIG"`,
raising a `KeyError` because the key is absent, which aborts application
startup with a
stack trace during initialization.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/cache_manager.py
**Line:** 239:243
**Comment:**
*Logic Error: The new inflight query cache initialization
unconditionally indexes `app.config["INFLIGHT_QUERY_STATE_CACHE_CONFIG"]` via
`_init_cache`, so if this optional config key is not defined (which will be
true on existing deployments that haven't added it yet), application startup
will crash with a `KeyError` even though the cache is not marked as required.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=25cc64eba2c9dc7490ea66890984f094b4a0f097c52c6e7fcdf5d6d86e56ec1b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38354&comment_hash=25cc64eba2c9dc7490ea66890984f094b4a0f097c52c6e7fcdf5d6d86e56ec1b&reaction=dislike'>👎</a>
--
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]
