You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/02/13 16:02:13 UTC
(superset) branch 3.1 updated (86794cbb5e -> 8831d60bbc)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
from 86794cbb5e fix(cache): remove unused webserver config & handle trailing slashes (#22849)
new dac73fe0cd feat(embedded+async queries): support async queries to work with embedded guest user (#26332)
new ab7f560d4b fix(plugin-chart-table): Prevent misalignment of totals and headers when scrollbar is visible (#26964)
new e3abdd5b6a fix: column values with NaN (#26946)
new 4df40bedc2 fix(tags): Improve support for tags with colons (#26965)
new 5a6109b2de fix(security manager): Users should not have access to all draft dashboards (#27015)
new c974daa0c7 fix: safer error message in alerts (#27019)
new d572af3073 fix(explore): allow free-form d3 format on custom column formatting (#27023)
new bb44099c68 fix(plugins): missing currency on small number format in table chart (#27041)
new 3d6dc9c280 fix: Exclude header controls from dashboard PDF export (#27068)
new 3c74a9b866 fix: Filters sidebar stretching dashboard height (#27069)
new f440a6a535 fix(drill): no rows returned (#27073)
new cde63c8c1e fix(big_number): white-space: nowrap to prevent wrapping (#27096)
new 4704380b2a build(deps): bump csstype from 2.6.9 to 3.1.3 in /superset-frontend (#26716)
new 10c9a7f0e2 fix: chart import validation (#26993)
new 534e8f394e fix: bump FAB to 4.3.11 (#27039)
new 8831d60bbc fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)
The 16 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
requirements/base.txt | 9 +-
setup.py | 4 +-
superset-frontend/package-lock.json | 18 ++-
.../superset-ui-chart-controls/src/types.ts | 1 +
.../packages/superset-ui-core/package.json | 2 +-
.../src/BigNumber/BigNumberViz.tsx | 1 +
.../src/DataTable/hooks/useSticky.tsx | 4 +
.../plugin-chart-table/src/utils/formatValue.ts | 6 +
.../plugin-chart-table/test/TableChart.test.tsx | 42 ++++++
.../src/components/Chart/DrillBy/DrillByModal.tsx | 11 +-
.../src/components/Chart/chartAction.js | 52 +++----
.../src/components/Chart/chartActions.test.js | 43 +++++-
superset-frontend/src/components/Tags/utils.tsx | 9 +-
.../DashboardBuilder/DashboardWrapper.tsx | 1 +
.../controls/ColumnConfigControl/constants.tsx | 1 +
superset-frontend/src/features/tags/tags.ts | 16 +-
superset-frontend/src/types/dom-to-pdf.d.ts | 1 +
superset-frontend/src/utils/downloadAsPdf.ts | 1 +
superset/async_events/async_query_manager.py | 26 +++-
superset/commands/chart/importers/v1/utils.py | 13 +-
superset/commands/dashboard/importers/v1/utils.py | 8 +-
superset/commands/dataset/importers/v1/utils.py | 7 +-
superset/commands/report/alert.py | 9 +-
superset/common/query_context_processor.py | 10 +-
superset/db_engine_specs/drill.py | 37 ++++-
superset/errors.py | 1 +
superset/jinja_context.py | 9 +-
superset/models/helpers.py | 7 +-
superset/security/manager.py | 72 +++++++--
superset/tasks/async_queries.py | 37 +++--
superset/utils/core.py | 9 ++
tests/integration_tests/charts/commands_tests.py | 2 +-
tests/integration_tests/conftest.py | 21 +--
.../integration_tests/dashboards/commands_tests.py | 2 +-
.../dashboards/security/security_rbac_tests.py | 24 +--
tests/integration_tests/datasets/commands_tests.py | 2 +-
tests/integration_tests/datasource/api_tests.py | 10 ++
tests/integration_tests/datasource_tests.py | 9 +-
tests/integration_tests/query_context_tests.py | 1 +
.../async_events/async_query_manager_tests.py | 79 +++++++++-
.../charts/commands/importers/v1/import_test.py | 164 ++++++++++++++++-----
.../commands/importers/v1/import_test.py | 152 ++++++++-----------
tests/unit_tests/security/manager_test.py | 140 ++++++++++++++++++
43 files changed, 810 insertions(+), 263 deletions(-)
(superset) 07/16: fix(explore): allow free-form d3 format on custom column formatting (#27023)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d572af307349762a92d29451661fe769d5ab22e5
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Thu Feb 8 09:27:59 2024 -0800
fix(explore): allow free-form d3 format on custom column formatting (#27023)
(cherry picked from commit fd06ff3745b0ce96ef2506e18b6d5f27d3eee045)
---
superset-frontend/packages/superset-ui-chart-controls/src/types.ts | 1 +
.../src/explore/components/controls/ColumnConfigControl/constants.tsx | 1 +
2 files changed, 2 insertions(+)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
index 9314f8d33f..83b711baaa 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
@@ -518,6 +518,7 @@ export type ControlFormItemSpec<T extends ControlType = ControlType> = {
debounceDelay?: number;
} & (T extends 'Select'
? {
+ allowNewOptions?: boolean;
options: any;
value?: string;
defaultValue?: string;
diff --git a/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx b/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx
index 684d8faf14..985fc2f47c 100644
--- a/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx
+++ b/superset-frontend/src/explore/components/controls/ColumnConfigControl/constants.tsx
@@ -42,6 +42,7 @@ export type SharedColumnConfigProp =
| 'currencyFormat';
const d3NumberFormat: ControlFormItemSpec<'Select'> = {
+ allowNewOptions: true,
controlType: 'Select',
label: t('D3 format'),
description: D3_FORMAT_DOCS,
(superset) 14/16: fix: chart import validation (#26993)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 10c9a7f0e253dd5b55f6336d2c64904ae4a6dfc5
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Tue Feb 6 12:14:02 2024 +0000
fix: chart import validation (#26993)
---
superset/commands/chart/importers/v1/utils.py | 13 +-
superset/commands/dashboard/importers/v1/utils.py | 8 +-
superset/commands/dataset/importers/v1/utils.py | 7 +-
superset/errors.py | 1 +
superset/jinja_context.py | 9 +-
superset/security/manager.py | 45 +++++-
superset/utils/core.py | 9 ++
tests/integration_tests/charts/commands_tests.py | 2 +-
.../integration_tests/dashboards/commands_tests.py | 2 +-
tests/integration_tests/datasets/commands_tests.py | 2 +-
.../charts/commands/importers/v1/import_test.py | 164 ++++++++++++++++-----
.../commands/importers/v1/import_test.py | 152 ++++++++-----------
tests/unit_tests/security/manager_test.py | 140 ++++++++++++++++++
13 files changed, 405 insertions(+), 149 deletions(-)
diff --git a/superset/commands/chart/importers/v1/utils.py b/superset/commands/chart/importers/v1/utils.py
index f905f8cc3a..2aac3ea9c4 100644
--- a/superset/commands/chart/importers/v1/utils.py
+++ b/superset/commands/chart/importers/v1/utils.py
@@ -20,7 +20,6 @@ import json
from inspect import isclass
from typing import Any
-from flask import g
from sqlalchemy.orm import Session
from superset import security_manager
@@ -28,7 +27,7 @@ from superset.commands.exceptions import ImportFailedError
from superset.migrations.shared.migrate_viz import processors
from superset.migrations.shared.migrate_viz.base import MigrateViz
from superset.models.slice import Slice
-from superset.utils.core import AnnotationType
+from superset.utils.core import AnnotationType, get_user
def filter_chart_annotations(chart_config: dict[str, Any]) -> None:
@@ -55,6 +54,12 @@ def import_chart(
can_write = ignore_permissions or security_manager.can_access("can_write", "Chart")
existing = session.query(Slice).filter_by(uuid=config["uuid"]).first()
if existing:
+ if overwrite and can_write and get_user():
+ if not security_manager.can_access_chart(existing):
+ raise ImportFailedError(
+ "A chart already exists and user doesn't "
+ "have permissions to overwrite it"
+ )
if not overwrite or not can_write:
return existing
config["id"] = existing.id
@@ -77,8 +82,8 @@ def import_chart(
if chart.id is None:
session.flush()
- if hasattr(g, "user") and g.user:
- chart.owners.append(g.user)
+ if user := get_user():
+ chart.owners.append(user)
return chart
diff --git a/superset/commands/dashboard/importers/v1/utils.py b/superset/commands/dashboard/importers/v1/utils.py
index 712161bc16..b8ac3144db 100644
--- a/superset/commands/dashboard/importers/v1/utils.py
+++ b/superset/commands/dashboard/importers/v1/utils.py
@@ -19,12 +19,12 @@ import json
import logging
from typing import Any
-from flask import g
from sqlalchemy.orm import Session
from superset import security_manager
from superset.commands.exceptions import ImportFailedError
from superset.models.dashboard import Dashboard
+from superset.utils.core import get_user
logger = logging.getLogger(__name__)
@@ -157,7 +157,7 @@ def import_dashboard(
)
existing = session.query(Dashboard).filter_by(uuid=config["uuid"]).first()
if existing:
- if overwrite and can_write and hasattr(g, "user") and g.user:
+ if overwrite and can_write and get_user():
if not security_manager.can_access_dashboard(existing):
raise ImportFailedError(
"A dashboard already exists and user doesn't "
@@ -191,7 +191,7 @@ def import_dashboard(
if dashboard.id is None:
session.flush()
- if hasattr(g, "user") and g.user:
- dashboard.owners.append(g.user)
+ if user := get_user():
+ dashboard.owners.append(user)
return dashboard
diff --git a/superset/commands/dataset/importers/v1/utils.py b/superset/commands/dataset/importers/v1/utils.py
index c145cc50f9..b0a681becf 100644
--- a/superset/commands/dataset/importers/v1/utils.py
+++ b/superset/commands/dataset/importers/v1/utils.py
@@ -22,7 +22,7 @@ from typing import Any
from urllib import request
import pandas as pd
-from flask import current_app, g
+from flask import current_app
from sqlalchemy import BigInteger, Boolean, Date, DateTime, Float, String, Text
from sqlalchemy.orm import Session
from sqlalchemy.orm.exc import MultipleResultsFound
@@ -33,6 +33,7 @@ from superset.commands.dataset.exceptions import DatasetForbiddenDataURI
from superset.commands.exceptions import ImportFailedError
from superset.connectors.sqla.models import SqlaTable
from superset.models.core import Database
+from superset.utils.core import get_user
logger = logging.getLogger(__name__)
@@ -176,8 +177,8 @@ def import_dataset(
if data_uri and (not table_exists or force_data):
load_data(data_uri, dataset, dataset.database, session)
- if hasattr(g, "user") and g.user:
- dataset.owners.append(g.user)
+ if user := get_user():
+ dataset.owners.append(user)
return dataset
diff --git a/superset/errors.py b/superset/errors.py
index 87cdf77b99..6be4f966ce 100644
--- a/superset/errors.py
+++ b/superset/errors.py
@@ -66,6 +66,7 @@ class SupersetErrorType(StrEnum):
MISSING_OWNERSHIP_ERROR = "MISSING_OWNERSHIP_ERROR"
USER_ACTIVITY_SECURITY_ACCESS_ERROR = "USER_ACTIVITY_SECURITY_ACCESS_ERROR"
DASHBOARD_SECURITY_ACCESS_ERROR = "DASHBOARD_SECURITY_ACCESS_ERROR"
+ CHART_SECURITY_ACCESS_ERROR = "CHART_SECURITY_ACCESS_ERROR"
# Other errors
BACKEND_TIMEOUT_ERROR = "BACKEND_TIMEOUT_ERROR"
diff --git a/superset/jinja_context.py b/superset/jinja_context.py
index 3b046b732e..54d1f54866 100644
--- a/superset/jinja_context.py
+++ b/superset/jinja_context.py
@@ -36,7 +36,7 @@ from superset.exceptions import SupersetTemplateException
from superset.extensions import feature_flag_manager
from superset.utils.core import (
convert_legacy_filters_into_adhoc,
- get_user_id,
+ get_user,
merge_extra_filters,
)
@@ -110,11 +110,10 @@ class ExtraCache:
:returns: The user ID
"""
- if hasattr(g, "user") and g.user:
- id_ = get_user_id()
+ if user := get_user():
if add_to_cache_keys:
- self.cache_key_wrapper(id_)
- return id_
+ self.cache_key_wrapper(user.id)
+ return user.id
return None
def current_username(self, add_to_cache_keys: bool = True) -> Optional[str]:
diff --git a/superset/security/manager.py b/superset/security/manager.py
index e11ab68e74..1cc289f877 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -85,6 +85,7 @@ if TYPE_CHECKING:
)
from superset.models.core import Database
from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
from superset.models.sql_lab import Query
from superset.sql_parse import Table
from superset.viz import BaseViz
@@ -422,6 +423,19 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
return True
+ def can_access_chart(self, chart: "Slice") -> bool:
+ """
+ Return True if the user can access the specified chart, False otherwise.
+ :param chart: The chart
+ :return: Whether the user can access the chart
+ """
+ try:
+ self.raise_for_access(chart=chart)
+ except SupersetSecurityException:
+ return False
+
+ return True
+
def get_dashboard_access_error_object( # pylint: disable=invalid-name
self,
dashboard: "Dashboard", # pylint: disable=unused-argument
@@ -439,6 +453,23 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
level=ErrorLevel.ERROR,
)
+ def get_chart_access_error_object( # pylint: disable=invalid-name
+ self,
+ dashboard: "Dashboard", # pylint: disable=unused-argument
+ ) -> SupersetError:
+ """
+ Return the error object for the denied Superset dashboard.
+
+ :param dashboard: The denied Superset dashboard
+ :returns: The error object
+ """
+
+ return SupersetError(
+ error_type=SupersetErrorType.CHART_SECURITY_ACCESS_ERROR,
+ message="You don't have access to this chart.",
+ level=ErrorLevel.ERROR,
+ )
+
@staticmethod
def get_datasource_access_error_msg(datasource: "BaseDatasource") -> str:
"""
@@ -1822,6 +1853,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
# pylint: disable=too-many-arguments,too-many-branches,too-many-locals,too-many-statements
self,
dashboard: Optional["Dashboard"] = None,
+ chart: Optional["Slice"] = None,
database: Optional["Database"] = None,
datasource: Optional["BaseDatasource"] = None,
query: Optional["Query"] = None,
@@ -2047,9 +2079,16 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
self.get_dashboard_access_error_object(dashboard)
)
- def get_user_by_username(
- self, username: str, session: Session = None
- ) -> Optional[User]:
+ if chart:
+ if self.is_admin() or self.is_owner(chart):
+ return
+
+ if chart.datasource and self.can_access_datasource(chart.datasource):
+ return
+
+ raise SupersetSecurityException(self.get_chart_access_error_object(chart))
+
+ def get_user_by_username(self, username: str, session: Session = None) -> Optional[User]:
"""
Retrieves a user by it's username case sensitive. Optional session parameter
utility method normally useful for celery tasks where the session
diff --git a/superset/utils/core.py b/superset/utils/core.py
index b9c24076a4..3d7f208532 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -1344,6 +1344,15 @@ def split_adhoc_filters_into_base_filters( # pylint: disable=invalid-name
form_data["filters"] = simple_where_filters
+def get_user() -> User | None:
+ """
+ Get the current user (if defined).
+
+ :returns: The current user
+ """
+ return g.user if hasattr(g, "user") else None
+
+
def get_username() -> str | None:
"""
Get username (if defined) associated with the current user.
diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py
index b6adf197f5..a72a716d17 100644
--- a/tests/integration_tests/charts/commands_tests.py
+++ b/tests/integration_tests/charts/commands_tests.py
@@ -171,7 +171,7 @@ class TestExportChartsCommand(SupersetTestCase):
class TestImportChartsCommand(SupersetTestCase):
- @patch("superset.commands.chart.importers.v1.utils.g")
+ @patch("superset.utils.core.g")
@patch("superset.security.manager.g")
def test_import_v1_chart(self, sm_g, utils_g):
"""Test that we can import a chart"""
diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py
index 175a8a3198..d173c48cf0 100644
--- a/tests/integration_tests/dashboards/commands_tests.py
+++ b/tests/integration_tests/dashboards/commands_tests.py
@@ -486,7 +486,7 @@ class TestImportDashboardsCommand(SupersetTestCase):
db.session.delete(dataset)
db.session.commit()
- @patch("superset.commands.dashboard.importers.v1.utils.g")
+ @patch("superset.utils.core.g")
@patch("superset.security.manager.g")
def test_import_v1_dashboard(self, sm_g, utils_g):
"""Test that we can import a dashboard"""
diff --git a/tests/integration_tests/datasets/commands_tests.py b/tests/integration_tests/datasets/commands_tests.py
index b45bbdb76d..7b6066a22a 100644
--- a/tests/integration_tests/datasets/commands_tests.py
+++ b/tests/integration_tests/datasets/commands_tests.py
@@ -339,7 +339,7 @@ class TestImportDatasetsCommand(SupersetTestCase):
db.session.delete(dataset)
db.session.commit()
- @patch("superset.commands.dataset.importers.v1.utils.g")
+ @patch("superset.utils.core.g")
@patch("superset.security.manager.g")
@pytest.mark.usefixtures("load_energy_table_with_slice")
def test_import_v1_dataset(self, sm_g, utils_g):
diff --git a/tests/unit_tests/charts/commands/importers/v1/import_test.py b/tests/unit_tests/charts/commands/importers/v1/import_test.py
index 903b7468ba..bcff3ee411 100644
--- a/tests/unit_tests/charts/commands/importers/v1/import_test.py
+++ b/tests/unit_tests/charts/commands/importers/v1/import_test.py
@@ -17,104 +17,130 @@
# pylint: disable=unused-argument, import-outside-toplevel, unused-import, invalid-name
import copy
+from collections.abc import Generator
import pytest
+from flask_appbuilder.security.sqla.models import Role, User
from pytest_mock import MockFixture
from sqlalchemy.orm.session import Session
+from superset import security_manager
+from superset.commands.chart.importers.v1.utils import import_chart
from superset.commands.exceptions import ImportFailedError
+from superset.connectors.sqla.models import Database, SqlaTable
+from superset.models.slice import Slice
+from superset.utils.core import override_user
+from tests.integration_tests.fixtures.importexport import chart_config
-def test_import_chart(mocker: MockFixture, session: Session) -> None:
+@pytest.fixture
+def session_with_data(session: Session) -> Generator[Session, None, None]:
+ engine = session.get_bind()
+ SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
+
+ dataset = SqlaTable(
+ table_name="test_table",
+ metrics=[],
+ main_dttm_col=None,
+ database=Database(database_name="my_database", sqlalchemy_uri="sqlite://"),
+ )
+ session.add(dataset)
+ session.flush()
+ slice = Slice(
+ id=1,
+ datasource_id=dataset.id,
+ datasource_type="table",
+ datasource_name="tmp_perm_table",
+ slice_name="slice_name",
+ uuid=chart_config["uuid"],
+ )
+ session.add(slice)
+ session.flush()
+
+ yield session
+ session.rollback()
+
+
+@pytest.fixture
+def session_with_schema(session: Session) -> Generator[Session, None, None]:
+ from superset.connectors.sqla.models import SqlaTable
+
+ engine = session.get_bind()
+ SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
+
+ yield session
+
+
+def test_import_chart(mocker: MockFixture, session_with_schema: Session) -> None:
"""
Test importing a chart.
"""
- from superset import security_manager
- from superset.commands.chart.importers.v1.utils import import_chart
- from superset.connectors.sqla.models import SqlaTable
- from superset.models.core import Database
- from superset.models.slice import Slice
- from tests.integration_tests.fixtures.importexport import chart_config
mocker.patch.object(security_manager, "can_access", return_value=True)
- engine = session.get_bind()
- Slice.metadata.create_all(engine) # pylint: disable=no-member
-
config = copy.deepcopy(chart_config)
config["datasource_id"] = 1
config["datasource_type"] = "table"
- chart = import_chart(session, config)
+ chart = import_chart(session_with_schema, config)
assert chart.slice_name == "Deck Path"
assert chart.viz_type == "deck_path"
assert chart.is_managed_externally is False
assert chart.external_url is None
+ # Assert that the can write to chart was checked
+ security_manager.can_access.assert_called_once_with("can_write", "Chart")
-def test_import_chart_managed_externally(mocker: MockFixture, session: Session) -> None:
+
+def test_import_chart_managed_externally(
+ mocker: MockFixture, session_with_schema: Session
+) -> None:
"""
Test importing a chart that is managed externally.
"""
- from superset import security_manager
- from superset.commands.chart.importers.v1.utils import import_chart
- from superset.connectors.sqla.models import SqlaTable
- from superset.models.core import Database
- from superset.models.slice import Slice
- from tests.integration_tests.fixtures.importexport import chart_config
-
mocker.patch.object(security_manager, "can_access", return_value=True)
- engine = session.get_bind()
- Slice.metadata.create_all(engine) # pylint: disable=no-member
-
config = copy.deepcopy(chart_config)
config["datasource_id"] = 1
config["datasource_type"] = "table"
config["is_managed_externally"] = True
config["external_url"] = "https://example.org/my_chart"
- chart = import_chart(session, config)
+ chart = import_chart(session_with_schema, config)
assert chart.is_managed_externally is True
assert chart.external_url == "https://example.org/my_chart"
+ # Assert that the can write to chart was checked
+ security_manager.can_access.assert_called_once_with("can_write", "Chart")
+
def test_import_chart_without_permission(
mocker: MockFixture,
- session: Session,
+ session_with_schema: Session,
) -> None:
"""
Test importing a chart when a user doesn't have permissions to create.
"""
- from superset import security_manager
- from superset.commands.chart.importers.v1.utils import import_chart
- from superset.connectors.sqla.models import SqlaTable
- from superset.models.core import Database
- from superset.models.slice import Slice
- from tests.integration_tests.fixtures.importexport import chart_config
-
mocker.patch.object(security_manager, "can_access", return_value=False)
- engine = session.get_bind()
- Slice.metadata.create_all(engine) # pylint: disable=no-member
-
config = copy.deepcopy(chart_config)
config["datasource_id"] = 1
config["datasource_type"] = "table"
with pytest.raises(ImportFailedError) as excinfo:
- import_chart(session, config)
+ import_chart(session_with_schema, config)
assert (
str(excinfo.value)
== "Chart doesn't exist and user doesn't have permission to create charts"
)
+ # Assert that the can write to chart was checked
+ security_manager.can_access.assert_called_once_with("can_write", "Chart")
-def test_filter_chart_annotations(mocker: MockFixture, session: Session) -> None:
+def test_filter_chart_annotations(session: Session) -> None:
"""
Test importing a chart.
"""
- from superset import security_manager
from superset.commands.chart.importers.v1.utils import filter_chart_annotations
from tests.integration_tests.fixtures.importexport import (
chart_config_with_mixed_annotations,
@@ -127,3 +153,67 @@ def test_filter_chart_annotations(mocker: MockFixture, session: Session) -> None
assert len(annotation_layers) == 1
assert all([al["annotationType"] == "FORMULA" for al in annotation_layers])
+
+
+def test_import_existing_chart_without_permission(
+ mocker: MockFixture,
+ session_with_data: Session,
+) -> None:
+ """
+ Test importing a chart when a user doesn't have permissions to modify.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ mocker.patch.object(security_manager, "can_access_chart", return_value=False)
+
+ slice = (
+ session_with_data.query(Slice)
+ .filter(Slice.uuid == chart_config["uuid"])
+ .one_or_none()
+ )
+
+ with override_user("admin"):
+ with pytest.raises(ImportFailedError) as excinfo:
+ import_chart(session_with_data, chart_config, overwrite=True)
+ assert (
+ str(excinfo.value)
+ == "A chart already exists and user doesn't have permissions to overwrite it"
+ )
+
+ # Assert that the can write to chart was checked
+ security_manager.can_access.assert_called_once_with("can_write", "Chart")
+ security_manager.can_access_chart.assert_called_once_with(slice)
+
+
+def test_import_existing_chart_with_permission(
+ mocker: MockFixture,
+ session_with_data: Session,
+) -> None:
+ """
+ Test importing a chart that exists when a user has access permission to that chart.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
+ mocker.patch.object(security_manager, "can_access_chart", return_value=True)
+
+ admin = User(
+ first_name="Alice",
+ last_name="Doe",
+ email="adoe@example.org",
+ username="admin",
+ roles=[Role(name="Admin")],
+ )
+
+ config = copy.deepcopy(chart_config)
+ config["datasource_id"] = 1
+ config["datasource_type"] = "table"
+
+ slice = (
+ session_with_data.query(Slice)
+ .filter(Slice.uuid == config["uuid"])
+ .one_or_none()
+ )
+
+ with override_user(admin):
+ import_chart(session_with_data, config, overwrite=True)
+ # Assert that the can write to chart was checked
+ security_manager.can_access.assert_called_once_with("can_write", "Chart")
+ security_manager.can_access_chart.assert_called_once_with(slice)
diff --git a/tests/unit_tests/dashboards/commands/importers/v1/import_test.py b/tests/unit_tests/dashboards/commands/importers/v1/import_test.py
index 190f0660f5..afbce49cd9 100644
--- a/tests/unit_tests/dashboards/commands/importers/v1/import_test.py
+++ b/tests/unit_tests/dashboards/commands/importers/v1/import_test.py
@@ -17,32 +17,57 @@
# pylint: disable=unused-argument, import-outside-toplevel, unused-import, invalid-name
import copy
+from collections.abc import Generator
import pytest
+from flask_appbuilder.security.sqla.models import Role, User
from pytest_mock import MockFixture
from sqlalchemy.orm.session import Session
+from superset import security_manager
+from superset.commands.dashboard.importers.v1.utils import import_dashboard
from superset.commands.exceptions import ImportFailedError
+from superset.models.dashboard import Dashboard
from superset.utils.core import override_user
+from tests.integration_tests.fixtures.importexport import dashboard_config
-def test_import_dashboard(mocker: MockFixture, session: Session) -> None:
- """
- Test importing a dashboard.
- """
- from superset import security_manager
- from superset.commands.dashboard.importers.v1.utils import import_dashboard
- from superset.models.slice import Slice
- from tests.integration_tests.fixtures.importexport import dashboard_config
+@pytest.fixture
+def session_with_data(session: Session) -> Generator[Session, None, None]:
+ engine = session.get_bind()
+ Dashboard.metadata.create_all(engine) # pylint: disable=no-member
+
+ dashboard = Dashboard(
+ id=100,
+ dashboard_title="Test dash",
+ slug=None,
+ slices=[],
+ published=True,
+ uuid=dashboard_config["uuid"],
+ )
+
+ session.add(dashboard)
+ session.flush()
+ yield session
+ session.rollback()
- mocker.patch.object(security_manager, "can_access", return_value=True)
+@pytest.fixture
+def session_with_schema(session: Session) -> Generator[Session, None, None]:
engine = session.get_bind()
- Slice.metadata.create_all(engine) # pylint: disable=no-member
+ Dashboard.metadata.create_all(engine) # pylint: disable=no-member
- config = copy.deepcopy(dashboard_config)
+ yield session
+ session.rollback()
+
+
+def test_import_dashboard(mocker: MockFixture, session_with_schema: Session) -> None:
+ """
+ Test importing a dashboard.
+ """
+ mocker.patch.object(security_manager, "can_access", return_value=True)
- dashboard = import_dashboard(session, config)
+ dashboard = import_dashboard(session_with_schema, dashboard_config)
assert dashboard.dashboard_title == "Test dash"
assert dashboard.description is None
assert dashboard.is_managed_externally is False
@@ -53,26 +78,18 @@ def test_import_dashboard(mocker: MockFixture, session: Session) -> None:
def test_import_dashboard_managed_externally(
mocker: MockFixture,
- session: Session,
+ session_with_schema: Session,
) -> None:
"""
Test importing a dashboard that is managed externally.
"""
- from superset import security_manager
- from superset.commands.dashboard.importers.v1.utils import import_dashboard
- from superset.models.slice import Slice
- from tests.integration_tests.fixtures.importexport import dashboard_config
-
mocker.patch.object(security_manager, "can_access", return_value=True)
- engine = session.get_bind()
- Slice.metadata.create_all(engine) # pylint: disable=no-member
-
config = copy.deepcopy(dashboard_config)
config["is_managed_externally"] = True
config["external_url"] = "https://example.org/my_dashboard"
- dashboard = import_dashboard(session, config)
+ dashboard = import_dashboard(session_with_schema, config)
assert dashboard.is_managed_externally is True
assert dashboard.external_url == "https://example.org/my_dashboard"
@@ -82,25 +99,15 @@ def test_import_dashboard_managed_externally(
def test_import_dashboard_without_permission(
mocker: MockFixture,
- session: Session,
+ session_with_schema: Session,
) -> None:
"""
Test importing a dashboard when a user doesn't have permissions to create.
"""
- from superset import security_manager
- from superset.commands.dashboard.importers.v1.utils import import_dashboard
- from superset.models.slice import Slice
- from tests.integration_tests.fixtures.importexport import dashboard_config
-
mocker.patch.object(security_manager, "can_access", return_value=False)
- engine = session.get_bind()
- Slice.metadata.create_all(engine) # pylint: disable=no-member
-
- config = copy.deepcopy(dashboard_config)
-
with pytest.raises(ImportFailedError) as excinfo:
- import_dashboard(session, config)
+ import_dashboard(session_with_schema, dashboard_config)
assert (
str(excinfo.value)
== "Dashboard doesn't exist and user doesn't have permission to create dashboards"
@@ -112,72 +119,43 @@ def test_import_dashboard_without_permission(
def test_import_existing_dashboard_without_permission(
mocker: MockFixture,
- session: Session,
+ session_with_data: Session,
) -> None:
"""
Test importing a dashboard when a user doesn't have permissions to create.
"""
- from superset import security_manager
- from superset.commands.dashboard.importers.v1.utils import g, import_dashboard
- from superset.models.dashboard import Dashboard
- from superset.models.slice import Slice
- from tests.integration_tests.fixtures.importexport import dashboard_config
-
mocker.patch.object(security_manager, "can_access", return_value=True)
mocker.patch.object(security_manager, "can_access_dashboard", return_value=False)
- mock_g = mocker.patch(
- "superset.commands.dashboard.importers.v1.utils.g"
- ) # Replace with the actual path to g
- mock_g.user = mocker.MagicMock(return_value=True)
-
- engine = session.get_bind()
- Slice.metadata.create_all(engine) # pylint: disable=no-member
- Dashboard.metadata.create_all(engine) # pylint: disable=no-member
- dashboard_obj = Dashboard(
- id=100,
- dashboard_title="Test dash",
- slug=None,
- slices=[],
- published=True,
- uuid="c4b28c4e-a1fe-4cf8-a5ac-d6f11d6fdd51",
+ dashboard = (
+ session_with_data.query(Dashboard)
+ .filter(Dashboard.uuid == dashboard_config["uuid"])
+ .one_or_none()
)
- session.add(dashboard_obj)
- session.flush()
- config = copy.deepcopy(dashboard_config)
- with pytest.raises(ImportFailedError) as excinfo:
- import_dashboard(session, config, overwrite=True)
- assert (
- str(excinfo.value)
- == "A dashboard already exists and user doesn't have permissions to overwrite it"
- )
+ with override_user("admin"):
+ with pytest.raises(ImportFailedError) as excinfo:
+ import_dashboard(session_with_data, dashboard_config, overwrite=True)
+ assert (
+ str(excinfo.value)
+ == "A dashboard already exists and user doesn't have permissions to overwrite it"
+ )
# Assert that the can write to dashboard was checked
security_manager.can_access.assert_called_once_with("can_write", "Dashboard")
- security_manager.can_access_dashboard.assert_called_once_with(dashboard_obj)
+ security_manager.can_access_dashboard.assert_called_once_with(dashboard)
def test_import_existing_dashboard_with_permission(
mocker: MockFixture,
- session: Session,
+ session_with_data: Session,
) -> None:
"""
- Test importing a dashboard when a user doesn't have permissions to create.
+ Test importing a dashboard that exists when a user has access permission to that dashboard.
"""
- from flask_appbuilder.security.sqla.models import Role, User
-
- from superset import security_manager
- from superset.commands.dashboard.importers.v1.utils import import_dashboard
- from superset.models.dashboard import Dashboard
- from tests.integration_tests.fixtures.importexport import dashboard_config
-
mocker.patch.object(security_manager, "can_access", return_value=True)
mocker.patch.object(security_manager, "can_access_dashboard", return_value=True)
- engine = session.get_bind()
- Dashboard.metadata.create_all(engine) # pylint: disable=no-member
-
admin = User(
first_name="Alice",
last_name="Doe",
@@ -186,20 +164,14 @@ def test_import_existing_dashboard_with_permission(
roles=[Role(name="Admin")],
)
- dashboard_obj = Dashboard(
- id=100,
- dashboard_title="Test dash",
- slug=None,
- slices=[],
- published=True,
- uuid="c4b28c4e-a1fe-4cf8-a5ac-d6f11d6fdd51",
+ dashboard = (
+ session_with_data.query(Dashboard)
+ .filter(Dashboard.uuid == dashboard_config["uuid"])
+ .one_or_none()
)
- session.add(dashboard_obj)
- session.flush()
- config = copy.deepcopy(dashboard_config)
with override_user(admin):
- import_dashboard(session, config, overwrite=True)
+ import_dashboard(session_with_data, dashboard_config, overwrite=True)
# Assert that the can write to dashboard was checked
security_manager.can_access.assert_called_once_with("can_write", "Dashboard")
- security_manager.can_access_dashboard.assert_called_once_with(dashboard_obj)
+ security_manager.can_access_dashboard.assert_called_once_with(dashboard)
diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py
index ad6e53e993..7d2b9153a3 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -16,12 +16,16 @@
# under the License.
import pytest
+from flask_appbuilder.security.sqla.models import Role, User
from pytest_mock import MockFixture
from superset.common.query_object import QueryObject
+from superset.connectors.sqla.models import Database, SqlaTable
from superset.exceptions import SupersetSecurityException
from superset.extensions import appbuilder
+from superset.models.slice import Slice
from superset.security.manager import SupersetSecurityManager
+from superset.utils.core import override_user
def test_security_manager(app_context: None) -> None:
@@ -164,3 +168,139 @@ def test_raise_for_access_query_default_schema(
== """You need access to the following tables: `public.ab_user`,
`all_database_access` or `all_datasource_access` permission"""
)
+
+
+def test_raise_for_access_chart_for_datasource_permission(
+ mocker: MockFixture,
+ app_context: None,
+) -> None:
+ """
+ Test that the security manager can raise an exception for chart access,
+ when the user does not have access to the chart datasource
+ """
+ sm = SupersetSecurityManager(appbuilder)
+ session = sm.get_session
+
+ engine = session.get_bind()
+ Slice.metadata.create_all(engine) # pylint: disable=no-member
+
+ alpha = User(
+ first_name="Alice",
+ last_name="Doe",
+ email="adoe@example.org",
+ username="admin",
+ roles=[Role(name="Alpha")],
+ )
+
+ dataset = SqlaTable(
+ table_name="test_table",
+ metrics=[],
+ main_dttm_col=None,
+ database=Database(database_name="my_database", sqlalchemy_uri="sqlite://"),
+ )
+ session.add(dataset)
+ session.flush()
+
+ slice = Slice(
+ id=1,
+ datasource_id=dataset.id,
+ datasource_type="table",
+ datasource_name="tmp_perm_table",
+ slice_name="slice_name",
+ )
+ session.add(slice)
+ session.flush()
+
+ mocker.patch.object(sm, "can_access_datasource", return_value=False)
+ with override_user(alpha):
+ with pytest.raises(SupersetSecurityException) as excinfo:
+ sm.raise_for_access(
+ chart=slice,
+ )
+ assert str(excinfo.value) == "You don't have access to this chart."
+
+ mocker.patch.object(sm, "can_access_datasource", return_value=True)
+ with override_user(alpha):
+ sm.raise_for_access(
+ chart=slice,
+ )
+
+
+def test_raise_for_access_chart_on_admin(
+ app_context: None,
+) -> None:
+ """
+ Test that the security manager can raise an exception for chart access,
+ when the user does not have access to the chart datasource
+ """
+ from flask_appbuilder.security.sqla.models import Role, User
+
+ from superset.models.slice import Slice
+ from superset.utils.core import override_user
+
+ sm = SupersetSecurityManager(appbuilder)
+ session = sm.get_session
+
+ engine = session.get_bind()
+ Slice.metadata.create_all(engine) # pylint: disable=no-member
+
+ admin = User(
+ first_name="Alice",
+ last_name="Doe",
+ email="adoe@example.org",
+ username="admin",
+ roles=[Role(name="Admin")],
+ )
+
+ slice = Slice(
+ id=1,
+ datasource_id=1,
+ datasource_type="table",
+ datasource_name="tmp_perm_table",
+ slice_name="slice_name",
+ )
+ session.add(slice)
+ session.flush()
+
+ with override_user(admin):
+ sm.raise_for_access(
+ chart=slice,
+ )
+
+
+def test_raise_for_access_chart_owner(
+ app_context: None,
+) -> None:
+ """
+ Test that the security manager can raise an exception for chart access,
+ when the user does not have access to the chart datasource
+ """
+ sm = SupersetSecurityManager(appbuilder)
+ session = sm.get_session
+
+ engine = session.get_bind()
+ Slice.metadata.create_all(engine) # pylint: disable=no-member
+
+ alpha = User(
+ first_name="Alice",
+ last_name="Doe",
+ email="adoe@example.org",
+ username="admin",
+ roles=[Role(name="Alpha")],
+ )
+
+ slice = Slice(
+ id=1,
+ datasource_id=1,
+ datasource_type="table",
+ datasource_name="tmp_perm_table",
+ slice_name="slice_name",
+ owners=[alpha],
+ )
+ session.add(slice)
+ session.flush()
+
+ with override_user(alpha):
+ sm.raise_for_access(
+ chart=slice,
+ )
(superset) 16/16: fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8831d60bbc5a428d3cd35384d23b3fc67c34d295
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Feb 9 21:49:33 2024 +0100
fix: Drill by with GLOBAL_ASYNC_QUERIES (#27066)
---
.../src/components/Chart/DrillBy/DrillByModal.tsx | 11 +++--
.../src/components/Chart/chartAction.js | 52 +++++++++++-----------
.../src/components/Chart/chartActions.test.js | 43 +++++++++++++++++-
superset/security/manager.py | 6 ++-
4 files changed, 81 insertions(+), 31 deletions(-)
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
index d1e8b39e0c..5053a76711 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx
@@ -54,11 +54,12 @@ import {
LOG_ACTIONS_DRILL_BY_MODAL_OPENED,
LOG_ACTIONS_FURTHER_DRILL_BY,
} from 'src/logger/LogUtils';
+import { getQuerySettings } from 'src/explore/exploreUtils';
import { Dataset, DrillByType } from '../types';
import DrillByChart from './DrillByChart';
import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu';
import { useContextMenu } from '../ChartContextMenu/useContextMenu';
-import { getChartDataRequest } from '../chartAction';
+import { getChartDataRequest, handleChartDataResponse } from '../chartAction';
import { useDisplayModeToggle } from './useDisplayModeToggle';
import {
DrillByBreadcrumb,
@@ -390,13 +391,17 @@ export default function DrillByModal({
useEffect(() => {
if (drilledFormData) {
+ const [useLegacyApi] = getQuerySettings(drilledFormData);
setIsChartDataLoading(true);
setChartDataResult(undefined);
getChartDataRequest({
formData: drilledFormData,
})
- .then(({ json }) => {
- setChartDataResult(json.result);
+ .then(({ response, json }) =>
+ handleChartDataResponse(response, json, useLegacyApi),
+ )
+ .then(queriesResponse => {
+ setChartDataResult(queriesResponse);
})
.catch(() => {
addDangerToast(t('Failed to load chart data.'));
diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js
index 8cd3785ae5..9e7f961abd 100644
--- a/superset-frontend/src/components/Chart/chartAction.js
+++ b/superset-frontend/src/components/Chart/chartAction.js
@@ -374,6 +374,29 @@ export function addChart(chart, key) {
return { type: ADD_CHART, chart, key };
}
+export function handleChartDataResponse(response, json, useLegacyApi) {
+ if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) {
+ // deal with getChartDataRequest transforming the response data
+ const result = 'result' in json ? json.result : json;
+ switch (response.status) {
+ case 200:
+ // Query results returned synchronously, meaning query was already cached.
+ return Promise.resolve(result);
+ case 202:
+ // Query is running asynchronously and we must await the results
+ if (useLegacyApi) {
+ return waitForAsyncData(result[0]);
+ }
+ return waitForAsyncData(result);
+ default:
+ throw new Error(
+ `Received unexpected response status (${response.status}) while fetching chart data`,
+ );
+ }
+ }
+ return json.result;
+}
+
export function exploreJSON(
formData,
force = false,
@@ -409,31 +432,11 @@ export function exploreJSON(
dispatch(chartUpdateStarted(controller, formData, key));
+ const [useLegacyApi] = getQuerySettings(formData);
const chartDataRequestCaught = chartDataRequest
- .then(({ response, json }) => {
- if (isFeatureEnabled(FeatureFlag.GLOBAL_ASYNC_QUERIES)) {
- // deal with getChartDataRequest transforming the response data
- const result = 'result' in json ? json.result : json;
- const [useLegacyApi] = getQuerySettings(formData);
- switch (response.status) {
- case 200:
- // Query results returned synchronously, meaning query was already cached.
- return Promise.resolve(result);
- case 202:
- // Query is running asynchronously and we must await the results
- if (useLegacyApi) {
- return waitForAsyncData(result[0]);
- }
- return waitForAsyncData(result);
- default:
- throw new Error(
- `Received unexpected response status (${response.status}) while fetching chart data`,
- );
- }
- }
-
- return json.result;
- })
+ .then(({ response, json }) =>
+ handleChartDataResponse(response, json, useLegacyApi),
+ )
.then(queriesResponse => {
queriesResponse.forEach(resultItem =>
dispatch(
@@ -492,7 +495,6 @@ export function exploreJSON(
});
// only retrieve annotations when calling the legacy API
- const [useLegacyApi] = getQuerySettings(formData);
const annotationLayers = useLegacyApi
? formData.annotation_layers || []
: [];
diff --git a/superset-frontend/src/components/Chart/chartActions.test.js b/superset-frontend/src/components/Chart/chartActions.test.js
index b3a6fed9f5..c2a58e6094 100644
--- a/superset-frontend/src/components/Chart/chartActions.test.js
+++ b/superset-frontend/src/components/Chart/chartActions.test.js
@@ -21,10 +21,12 @@ import fetchMock from 'fetch-mock';
import sinon from 'sinon';
import * as chartlib from '@superset-ui/core';
-import { SupersetClient } from '@superset-ui/core';
+import { FeatureFlag, SupersetClient } from '@superset-ui/core';
import { LOG_EVENT } from 'src/logger/actions';
import * as exploreUtils from 'src/explore/exploreUtils';
import * as actions from 'src/components/Chart/chartAction';
+import * as asyncEvent from 'src/middleware/asyncEvent';
+import { handleChartDataResponse } from 'src/components/Chart/chartAction';
describe('chart actions', () => {
const MOCK_URL = '/mockURL';
@@ -33,6 +35,7 @@ describe('chart actions', () => {
let getChartDataUriStub;
let metadataRegistryStub;
let buildQueryRegistryStub;
+ let waitForAsyncDataStub;
let fakeMetadata;
const setupDefaultFetchMock = () => {
@@ -66,6 +69,9 @@ describe('chart actions', () => {
result_format: 'json',
}),
}));
+ waitForAsyncDataStub = sinon
+ .stub(asyncEvent, 'waitForAsyncData')
+ .callsFake(data => Promise.resolve(data));
});
afterEach(() => {
@@ -74,6 +80,11 @@ describe('chart actions', () => {
fetchMock.resetHistory();
metadataRegistryStub.restore();
buildQueryRegistryStub.restore();
+ waitForAsyncDataStub.restore();
+
+ global.featureFlags = {
+ [FeatureFlag.GlobalAsyncQueries]: false,
+ };
});
describe('v1 API', () => {
@@ -114,6 +125,36 @@ describe('chart actions', () => {
expect(fetchMock.calls(mockBigIntUrl)).toHaveLength(1);
expect(json.value.toString()).toEqual(expectedBigNumber);
});
+
+ it('handleChartDataResponse should return result if GlobalAsyncQueries flag is disabled', async () => {
+ const result = await handleChartDataResponse(
+ { status: 200 },
+ { result: [1, 2, 3] },
+ );
+ expect(result).toEqual([1, 2, 3]);
+ });
+
+ it('handleChartDataResponse should handle responses when GlobalAsyncQueries flag is enabled and results are returned synchronously', async () => {
+ global.featureFlags = {
+ [FeatureFlag.GlobalAsyncQueries]: true,
+ };
+ const result = await handleChartDataResponse(
+ { status: 200 },
+ { result: [1, 2, 3] },
+ );
+ expect(result).toEqual([1, 2, 3]);
+ });
+
+ it('handleChartDataResponse should handle responses when GlobalAsyncQueries flag is enabled and query is running asynchronously', async () => {
+ global.featureFlags = {
+ [FeatureFlag.GlobalAsyncQueries]: true,
+ };
+ const result = await handleChartDataResponse(
+ { status: 202 },
+ { result: [1, 2, 3] },
+ );
+ expect(result).toEqual([1, 2, 3]);
+ });
});
describe('legacy API', () => {
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 1cc289f877..e6eb77e645 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -453,7 +453,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
level=ErrorLevel.ERROR,
)
- def get_chart_access_error_object( # pylint: disable=invalid-name
+ def get_chart_access_error_object(
self,
dashboard: "Dashboard", # pylint: disable=unused-argument
) -> SupersetError:
@@ -2088,7 +2088,9 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
raise SupersetSecurityException(self.get_chart_access_error_object(chart))
- def get_user_by_username(self, username: str, session: Session = None) -> Optional[User]:
+ def get_user_by_username(
+ self, username: str, session: Session = None
+ ) -> Optional[User]:
"""
Retrieves a user by it's username case sensitive. Optional session parameter
utility method normally useful for celery tasks where the session
(superset) 02/16: fix(plugin-chart-table): Prevent misalignment of totals and headers when scrollbar is visible (#26964)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit ab7f560d4b7eb09618bf9ac02fd394b7d272d71f
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Thu Feb 1 18:27:22 2024 +0100
fix(plugin-chart-table): Prevent misalignment of totals and headers when scrollbar is visible (#26964)
(cherry picked from commit e6d2fb6fdfa4d741de16b322bdc4bd01fb559413)
---
.../plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx
index 067d071ee1..e154a521f6 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx
@@ -226,6 +226,7 @@ function StickyWrap({
height: maxHeight,
overflow: 'auto',
visibility: 'hidden',
+ scrollbarGutter: 'stable',
}}
>
{React.cloneElement(table, {}, theadWithRef, tbody, tfootWithRef)}
@@ -252,6 +253,7 @@ function StickyWrap({
ref={scrollHeaderRef}
style={{
overflow: 'hidden',
+ scrollbarGutter: 'stable',
}}
>
{React.cloneElement(
@@ -270,6 +272,7 @@ function StickyWrap({
ref={scrollFooterRef}
style={{
overflow: 'hidden',
+ scrollbarGutter: 'stable',
}}
>
{React.cloneElement(
@@ -297,6 +300,7 @@ function StickyWrap({
style={{
height: bodyHeight,
overflow: 'auto',
+ scrollbarGutter: 'stable',
}}
onScroll={sticky.hasHorizontalScroll ? onScroll : undefined}
>
(superset) 08/16: fix(plugins): missing currency on small number format in table chart (#27041)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit bb44099c68a004c93d4328dc66d036ae036f8d7f
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Thu Feb 8 15:24:24 2024 -0800
fix(plugins): missing currency on small number format in table chart (#27041)
(cherry picked from commit 6f402991e54ae6ab0c6c98613d7e831c7f847f54)
---
.../plugin-chart-table/src/utils/formatValue.ts | 6 ++++
.../plugin-chart-table/test/TableChart.test.tsx | 42 ++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/superset-frontend/plugins/plugin-chart-table/src/utils/formatValue.ts b/superset-frontend/plugins/plugin-chart-table/src/utils/formatValue.ts
index 139f92336c..043fa6b59d 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/utils/formatValue.ts
+++ b/superset-frontend/plugins/plugin-chart-table/src/utils/formatValue.ts
@@ -17,6 +17,7 @@
* under the License.
*/
import {
+ CurrencyFormatter,
DataRecordValue,
GenericDataType,
getNumberFormatter,
@@ -64,6 +65,11 @@ export function formatColumnValue(
const smallNumberFormatter =
config.d3SmallNumberFormat === undefined
? formatter
+ : config.currencyFormat
+ ? new CurrencyFormatter({
+ d3Format: config.d3SmallNumberFormat,
+ currency: config.currencyFormat,
+ })
: getNumberFormatter(config.d3SmallNumberFormat);
return formatValue(
isNumber && typeof value === 'number' && Math.abs(value) < 1
diff --git a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
index d6998476ba..52cfab1662 100644
--- a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
@@ -166,6 +166,48 @@ describe('plugin-chart-table', () => {
expect(cells[2]).toHaveTextContent('$ 0');
});
+ it('render small formatted data with currencies', () => {
+ const props = transformProps({
+ ...testData.raw,
+ rawFormData: {
+ ...testData.raw.rawFormData,
+ column_config: {
+ num: {
+ d3SmallNumberFormat: '.2r',
+ currencyFormat: { symbol: 'USD', symbolPosition: 'prefix' },
+ },
+ },
+ },
+ queriesData: [
+ {
+ ...testData.raw.queriesData[0],
+ data: [
+ {
+ num: 1234,
+ },
+ {
+ num: 0.5,
+ },
+ {
+ num: 0.61234,
+ },
+ ],
+ },
+ ],
+ });
+ render(
+ ProviderWrapper({
+ children: <TableChart {...props} sticky={false} />,
+ }),
+ );
+ const cells = document.querySelectorAll('td');
+
+ expect(document.querySelectorAll('th')[0]).toHaveTextContent('num');
+ expect(cells[0]).toHaveTextContent('$ 1.23k');
+ expect(cells[1]).toHaveTextContent('$ 0.50');
+ expect(cells[2]).toHaveTextContent('$ 0.61');
+ });
+
it('render empty data', () => {
wrap.setProps({ ...transformProps(testData.empty), sticky: false });
tree = wrap.render();
(superset) 03/16: fix: column values with NaN (#26946)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit e3abdd5b6ac84ecae593274049eb094b49f83ab7
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Fri Feb 2 12:56:02 2024 -0500
fix: column values with NaN (#26946)
(cherry picked from commit d8a98475036a4fba28b3d3eb508b3d1f3f5072aa)
---
superset/models/helpers.py | 7 ++++++-
tests/integration_tests/conftest.py | 21 +++++++++++----------
tests/integration_tests/datasource/api_tests.py | 10 ++++++++++
tests/integration_tests/datasource_tests.py | 9 ++++++++-
4 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 65d0b53728..4ff206882e 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -1341,7 +1341,10 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
return and_(*l)
def values_for_column(
- self, column_name: str, limit: int = 10000, denormalize_column: bool = False
+ self,
+ column_name: str,
+ limit: int = 10000,
+ denormalize_column: bool = False,
) -> list[Any]:
# denormalize column name before querying for values
# unless disabled in the dataset configuration
@@ -1379,6 +1382,8 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
sql = self.mutate_query_from_config(sql)
df = pd.read_sql_query(sql=sql, con=engine)
+ # replace NaN with None to ensure it can be serialized to JSON
+ df = df.replace({np.nan: None})
return df["column_values"].to_list()
def get_timestamp_expression(
diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py
index 3e6aa96307..b90416587c 100644
--- a/tests/integration_tests/conftest.py
+++ b/tests/integration_tests/conftest.py
@@ -296,25 +296,25 @@ def virtual_dataset():
dataset = SqlaTable(
table_name="virtual_dataset",
sql=(
- "SELECT 0 as col1, 'a' as col2, 1.0 as col3, NULL as col4, '2000-01-01 00:00:00' as col5 "
+ "SELECT 0 as col1, 'a' as col2, 1.0 as col3, NULL as col4, '2000-01-01 00:00:00' as col5, 1 as col6 "
"UNION ALL "
- "SELECT 1, 'b', 1.1, NULL, '2000-01-02 00:00:00' "
+ "SELECT 1, 'b', 1.1, NULL, '2000-01-02 00:00:00', NULL "
"UNION ALL "
- "SELECT 2 as col1, 'c' as col2, 1.2, NULL, '2000-01-03 00:00:00' "
+ "SELECT 2 as col1, 'c' as col2, 1.2, NULL, '2000-01-03 00:00:00', 3 "
"UNION ALL "
- "SELECT 3 as col1, 'd' as col2, 1.3, NULL, '2000-01-04 00:00:00' "
+ "SELECT 3 as col1, 'd' as col2, 1.3, NULL, '2000-01-04 00:00:00', 4 "
"UNION ALL "
- "SELECT 4 as col1, 'e' as col2, 1.4, NULL, '2000-01-05 00:00:00' "
+ "SELECT 4 as col1, 'e' as col2, 1.4, NULL, '2000-01-05 00:00:00', 5 "
"UNION ALL "
- "SELECT 5 as col1, 'f' as col2, 1.5, NULL, '2000-01-06 00:00:00' "
+ "SELECT 5 as col1, 'f' as col2, 1.5, NULL, '2000-01-06 00:00:00', 6 "
"UNION ALL "
- "SELECT 6 as col1, 'g' as col2, 1.6, NULL, '2000-01-07 00:00:00' "
+ "SELECT 6 as col1, 'g' as col2, 1.6, NULL, '2000-01-07 00:00:00', 7 "
"UNION ALL "
- "SELECT 7 as col1, 'h' as col2, 1.7, NULL, '2000-01-08 00:00:00' "
+ "SELECT 7 as col1, 'h' as col2, 1.7, NULL, '2000-01-08 00:00:00', 8 "
"UNION ALL "
- "SELECT 8 as col1, 'i' as col2, 1.8, NULL, '2000-01-09 00:00:00' "
+ "SELECT 8 as col1, 'i' as col2, 1.8, NULL, '2000-01-09 00:00:00', 9 "
"UNION ALL "
- "SELECT 9 as col1, 'j' as col2, 1.9, NULL, '2000-01-10 00:00:00' "
+ "SELECT 9 as col1, 'j' as col2, 1.9, NULL, '2000-01-10 00:00:00', 10"
),
database=get_example_database(),
)
@@ -324,6 +324,7 @@ def virtual_dataset():
TableColumn(column_name="col4", type="VARCHAR(255)", table=dataset)
# Different database dialect datetime type is not consistent, so temporarily use varchar
TableColumn(column_name="col5", type="VARCHAR(255)", table=dataset)
+ TableColumn(column_name="col6", type="INTEGER", table=dataset)
SqlMetric(metric_name="count", expression="count(*)", table=dataset)
db.session.add(dataset)
diff --git a/tests/integration_tests/datasource/api_tests.py b/tests/integration_tests/datasource/api_tests.py
index b6a6af105d..2605ec399f 100644
--- a/tests/integration_tests/datasource/api_tests.py
+++ b/tests/integration_tests/datasource/api_tests.py
@@ -72,6 +72,16 @@ class TestDatasourceApi(SupersetTestCase):
response = json.loads(rv.data.decode("utf-8"))
self.assertEqual(response["result"], [None])
+ @pytest.mark.usefixtures("app_context", "virtual_dataset")
+ def test_get_column_values_integers_with_nulls(self):
+ self.login(username="admin")
+ table = self.get_virtual_dataset()
+ rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col6/values/")
+ self.assertEqual(rv.status_code, 200)
+ response = json.loads(rv.data.decode("utf-8"))
+ for val in [1, None, 3, 4, 5, 6, 7, 8, 9, 10]:
+ assert val in response["result"]
+
@pytest.mark.usefixtures("app_context", "virtual_dataset")
def test_get_column_values_invalid_datasource_type(self):
self.login(username="admin")
diff --git a/tests/integration_tests/datasource_tests.py b/tests/integration_tests/datasource_tests.py
index 5ab81b58d1..d5f548f204 100644
--- a/tests/integration_tests/datasource_tests.py
+++ b/tests/integration_tests/datasource_tests.py
@@ -602,7 +602,14 @@ def test_get_samples_with_filters(test_client, login_as_admin, virtual_dataset):
},
)
assert rv.status_code == 200
- assert rv.json["result"]["colnames"] == ["col1", "col2", "col3", "col4", "col5"]
+ assert rv.json["result"]["colnames"] == [
+ "col1",
+ "col2",
+ "col3",
+ "col4",
+ "col5",
+ "col6",
+ ]
assert rv.json["result"]["rowcount"] == 1
# empty results
(superset) 06/16: fix: safer error message in alerts (#27019)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit c974daa0c7f05bdf8a5e59897c9a2feaaf94bc4e
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Wed Feb 7 14:58:51 2024 -0500
fix: safer error message in alerts (#27019)
(cherry picked from commit 686ce33ea5017aad4cca18a6409c00f6b366dcf4)
---
superset/commands/report/alert.py | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/superset/commands/report/alert.py b/superset/commands/report/alert.py
index 68013a2c00..fc1be14e02 100644
--- a/superset/commands/report/alert.py
+++ b/superset/commands/report/alert.py
@@ -150,7 +150,7 @@ class AlertCommand(BaseCommand):
rendered_sql, ALERT_SQL_LIMIT
)
- _, username = get_executor(
+ executor, username = get_executor( # pylint: disable=unused-variable
executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"],
model=self._report_schedule,
)
@@ -169,7 +169,12 @@ class AlertCommand(BaseCommand):
logger.warning("A timeout occurred while executing the alert query: %s", ex)
raise AlertQueryTimeout() from ex
except Exception as ex:
- raise AlertQueryError(message=str(ex)) from ex
+ logger.exception("An error occurred when running alert query")
+ # The exception message here can reveal to much information to malicious
+ # users, so we raise a generic message.
+ raise AlertQueryError(
+ message=_("An error occurred when running alert query")
+ ) from ex
def validate(self) -> None:
"""
(superset) 04/16: fix(tags): Improve support for tags with colons (#26965)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4df40bedc28c162eb62504a17cb8386a1c873c50
Author: Vitor Avila <96...@users.noreply.github.com>
AuthorDate: Wed Feb 7 13:20:28 2024 -0300
fix(tags): Improve support for tags with colons (#26965)
(cherry picked from commit e437356013adc8beb2eca39a31beca6ba56f4c23)
---
superset-frontend/src/components/Tags/utils.tsx | 9 +++++----
superset-frontend/src/features/tags/tags.ts | 16 +++++++---------
2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/superset-frontend/src/components/Tags/utils.tsx b/superset-frontend/src/components/Tags/utils.tsx
index 2dd38e6236..ec08a3b126 100644
--- a/superset-frontend/src/components/Tags/utils.tsx
+++ b/superset-frontend/src/components/Tags/utils.tsx
@@ -56,7 +56,10 @@ export const loadTags = async (
) => {
const searchColumn = 'name';
const query = rison.encode({
- filters: [{ col: searchColumn, opr: 'ct', value: search }],
+ filters: [
+ { col: searchColumn, opr: 'ct', value: search },
+ { col: 'type', opr: 'custom_tag', value: true },
+ ],
page,
page_size: pageSize,
order_column: searchColumn,
@@ -78,9 +81,7 @@ export const loadTags = async (
const data: {
label: string;
value: string | number;
- }[] = response.json.result
- .filter((item: Tag & { table_name: string }) => item.type === 1)
- .map(tagToSelectOption);
+ }[] = response.json.result.map(tagToSelectOption);
return {
data,
totalCount: response.json.count,
diff --git a/superset-frontend/src/features/tags/tags.ts b/superset-frontend/src/features/tags/tags.ts
index db172681cb..c1f5c38151 100644
--- a/superset-frontend/src/features/tags/tags.ts
+++ b/superset-frontend/src/features/tags/tags.ts
@@ -47,10 +47,15 @@ const map_object_type_to_id = (objectType: string) => {
};
export function fetchAllTags(
+ // fetch all tags (excluding system tags)
callback: (json: JsonObject) => void,
error: (response: Response) => void,
) {
- SupersetClient.get({ endpoint: `/api/v1/tag` })
+ SupersetClient.get({
+ endpoint: `/api/v1/tag/?q=${rison.encode({
+ filters: [{ col: 'type', opr: 'custom_tag', value: true }],
+ })}`,
+ })
.then(({ json }) => callback(json))
.catch(response => error(response));
}
@@ -89,11 +94,7 @@ export function fetchTags(
endpoint: `/api/v1/${objectType}/${objectId}`,
})
.then(({ json }) =>
- callback(
- json.result.tags.filter(
- (tag: Tag) => tag.name.indexOf(':') === -1 || includeTypes,
- ),
- ),
+ callback(json.result.tags.filter((tag: Tag) => tag.type === 1)),
)
.catch(response => error(response));
}
@@ -163,9 +164,6 @@ export function addTag(
if (objectType === undefined || objectId === undefined) {
throw new Error('Need to specify objectType and objectId');
}
- if (tag.indexOf(':') !== -1 && !includeTypes) {
- return;
- }
const objectTypeId = map_object_type_to_id(objectType);
SupersetClient.post({
endpoint: `/api/v1/tag/${objectTypeId}/${objectId}/`,
(superset) 10/16: fix: Filters sidebar stretching dashboard height (#27069)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3c74a9b866daba89920677b34b0df1e395516674
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Feb 9 18:16:39 2024 +0100
fix: Filters sidebar stretching dashboard height (#27069)
(cherry picked from commit 3f91bdb40d76539e953dd9205481459f6b2ae082)
---
.../src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx | 1 +
1 file changed, 1 insertion(+)
diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
index f39c7ed630..5bb193de1b 100644
--- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
+++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
@@ -25,6 +25,7 @@ import classNames from 'classnames';
const StyledDiv = styled.div`
${({ theme }) => css`
+ position: relative;
display: grid;
grid-template-columns: auto 1fr;
grid-template-rows: auto 1fr;
(superset) 12/16: fix(big_number): white-space: nowrap to prevent wrapping (#27096)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit cde63c8c1e741eac5cf4ae665fb52f17d237c601
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Mon Feb 12 19:09:13 2024 -0800
fix(big_number): white-space: nowrap to prevent wrapping (#27096)
(cherry picked from commit 4796484190010275c037595c79b01d281d09ff60)
---
.../plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx | 1 +
1 file changed, 1 insertion(+)
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx
index 112e21657f..8a95a81d5f 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx
@@ -354,6 +354,7 @@ export default styled(BigNumberVis)`
.header-line {
position: relative;
line-height: 1em;
+ white-space: nowrap;
span {
position: absolute;
bottom: 0;
(superset) 09/16: fix: Exclude header controls from dashboard PDF export (#27068)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3d6dc9c280a67f3daac5df284de4847fde333344
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Feb 9 18:02:05 2024 +0100
fix: Exclude header controls from dashboard PDF export (#27068)
(cherry picked from commit 005cee023b7c312d51e0f10629834d53dab4c60a)
---
superset-frontend/src/types/dom-to-pdf.d.ts | 1 +
superset-frontend/src/utils/downloadAsPdf.ts | 1 +
2 files changed, 2 insertions(+)
diff --git a/superset-frontend/src/types/dom-to-pdf.d.ts b/superset-frontend/src/types/dom-to-pdf.d.ts
index 19ecce85b4..bc884fd43a 100644
--- a/superset-frontend/src/types/dom-to-pdf.d.ts
+++ b/superset-frontend/src/types/dom-to-pdf.d.ts
@@ -9,6 +9,7 @@ declare module 'dom-to-pdf' {
filename: string;
image: Image;
html2canvas: object;
+ excludeClassNames?: string[];
}
const domToPdf = (
diff --git a/superset-frontend/src/utils/downloadAsPdf.ts b/superset-frontend/src/utils/downloadAsPdf.ts
index eebca66b8b..5367efc724 100644
--- a/superset-frontend/src/utils/downloadAsPdf.ts
+++ b/superset-frontend/src/utils/downloadAsPdf.ts
@@ -61,6 +61,7 @@ export default function downloadAsPdf(
filename: `${generateFileStem(description)}.pdf`,
image: { type: 'jpeg', quality: 1 },
html2canvas: { scale: 2 },
+ excludeClassNames: ['header-controls'],
};
return domToPdf(elementToPrint, options)
.then(() => {
(superset) 01/16: feat(embedded+async queries): support async queries to work with embedded guest user (#26332)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit dac73fe0cd0838576d08d4b6dbb4aaadc6f257a3
Author: Zef Lin <ze...@preset.io>
AuthorDate: Mon Jan 8 17:11:45 2024 -0800
feat(embedded+async queries): support async queries to work with embedded guest user (#26332)
(cherry picked from commit efdeb9df0550458363e1c84850770012f501c9fb)
---
superset/async_events/async_query_manager.py | 26 ++++++-
superset/common/query_context_processor.py | 10 ++-
superset/tasks/async_queries.py | 37 ++++++----
tests/integration_tests/query_context_tests.py | 1 +
.../async_events/async_query_manager_tests.py | 79 +++++++++++++++++++++-
5 files changed, 134 insertions(+), 19 deletions(-)
diff --git a/superset/async_events/async_query_manager.py b/superset/async_events/async_query_manager.py
index 94941541fb..32cf247cf3 100644
--- a/superset/async_events/async_query_manager.py
+++ b/superset/async_events/async_query_manager.py
@@ -191,9 +191,14 @@ class AsyncQueryManager:
force: Optional[bool] = False,
user_id: Optional[int] = None,
) -> dict[str, Any]:
+ # pylint: disable=import-outside-toplevel
+ from superset import security_manager
+
job_metadata = self.init_job(channel_id, user_id)
self._load_explore_json_into_cache_job.delay(
- job_metadata,
+ {**job_metadata, "guest_token": guest_user.guest_token}
+ if (guest_user := security_manager.get_current_guest_user_if_guest())
+ else job_metadata,
form_data,
response_type,
force,
@@ -201,10 +206,25 @@ class AsyncQueryManager:
return job_metadata
def submit_chart_data_job(
- self, channel_id: str, form_data: dict[str, Any], user_id: Optional[int]
+ self,
+ channel_id: str,
+ form_data: dict[str, Any],
+ user_id: Optional[int] = None,
) -> dict[str, Any]:
+ # pylint: disable=import-outside-toplevel
+ from superset import security_manager
+
+ # if it's guest user, we want to pass the guest token to the celery task
+ # chart data cache key is calculated based on the current user
+ # this way we can keep the cache key consistent between sync and async command
+ # so that it can be looked up consistently
job_metadata = self.init_job(channel_id, user_id)
- self._load_chart_data_into_cache_job.delay(job_metadata, form_data)
+ self._load_chart_data_into_cache_job.delay(
+ {**job_metadata, "guest_token": guest_user.guest_token}
+ if (guest_user := security_manager.get_current_guest_user_if_guest())
+ else job_metadata,
+ form_data,
+ )
return job_metadata
def read_events(
diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py
index 5b1414d53b..d8b5bea4bb 100644
--- a/superset/common/query_context_processor.py
+++ b/superset/common/query_context_processor.py
@@ -600,7 +600,15 @@ class QueryContextProcessor:
set_and_log_cache(
cache_manager.cache,
cache_key,
- {"data": self._query_context.cache_values},
+ {
+ "data": {
+ # setting form_data into query context cache value as well
+ # so that it can be used to reconstruct form_data field
+ # for query context object when reading from cache
+ "form_data": self._query_context.form_data,
+ **self._query_context.cache_values,
+ },
+ },
self.get_cache_timeout(),
)
return_value["cache_key"] = cache_key # type: ignore
diff --git a/superset/tasks/async_queries.py b/superset/tasks/async_queries.py
index 61970ca1f3..b804847cd8 100644
--- a/superset/tasks/async_queries.py
+++ b/superset/tasks/async_queries.py
@@ -22,6 +22,7 @@ from typing import Any, cast, TYPE_CHECKING
from celery.exceptions import SoftTimeLimitExceeded
from flask import current_app, g
+from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.charts.schemas import ChartDataQueryContextSchema
@@ -58,6 +59,20 @@ def _create_query_context_from_form(form_data: dict[str, Any]) -> QueryContext:
raise error
+def _load_user_from_job_metadata(job_metadata: dict[str, Any]) -> User:
+ if user_id := job_metadata.get("user_id"):
+ # logged in user
+ user = security_manager.get_user_by_id(user_id)
+ elif guest_token := job_metadata.get("guest_token"):
+ # embedded guest user
+ user = security_manager.get_guest_user_from_token(guest_token)
+ del job_metadata["guest_token"]
+ else:
+ # default to anonymous user if no user is found
+ user = security_manager.get_anonymous_user()
+ return user
+
+
@celery_app.task(name="load_chart_data_into_cache", soft_time_limit=query_timeout)
def load_chart_data_into_cache(
job_metadata: dict[str, Any],
@@ -66,12 +81,7 @@ def load_chart_data_into_cache(
# pylint: disable=import-outside-toplevel
from superset.commands.chart.data.get_data_command import ChartDataCommand
- user = (
- security_manager.get_user_by_id(job_metadata.get("user_id"))
- or security_manager.get_anonymous_user()
- )
-
- with override_user(user, force=False):
+ with override_user(_load_user_from_job_metadata(job_metadata), force=False):
try:
set_form_data(form_data)
query_context = _create_query_context_from_form(form_data)
@@ -106,12 +116,7 @@ def load_explore_json_into_cache( # pylint: disable=too-many-locals
) -> None:
cache_key_prefix = "ejr-" # ejr: explore_json request
- user = (
- security_manager.get_user_by_id(job_metadata.get("user_id"))
- or security_manager.get_anonymous_user()
- )
-
- with override_user(user, force=False):
+ with override_user(_load_user_from_job_metadata(job_metadata), force=False):
try:
set_form_data(form_data)
datasource_id, datasource_type = get_datasource_info(None, None, form_data)
@@ -140,7 +145,13 @@ def load_explore_json_into_cache( # pylint: disable=too-many-locals
"response_type": response_type,
}
cache_key = generate_cache_key(cache_value, cache_key_prefix)
- set_and_log_cache(cache_manager.cache, cache_key, cache_value)
+ cache_instance = cache_manager.cache
+ cache_timeout = (
+ cache_instance.cache.default_timeout if cache_instance.cache else None
+ )
+ set_and_log_cache(
+ cache_instance, cache_key, cache_value, cache_timeout=cache_timeout
+ )
result_url = f"/superset/explore_json/data/{cache_key}"
async_query_manager.update_job(
job_metadata,
diff --git a/tests/integration_tests/query_context_tests.py b/tests/integration_tests/query_context_tests.py
index 8c2082d1c4..30cd160d7e 100644
--- a/tests/integration_tests/query_context_tests.py
+++ b/tests/integration_tests/query_context_tests.py
@@ -121,6 +121,7 @@ class TestQueryContext(SupersetTestCase):
cached = cache_manager.cache.get(cache_key)
assert cached is not None
+ assert "form_data" in cached["data"]
rehydrated_qc = ChartDataQueryContextSchema().load(cached["data"])
rehydrated_qo = rehydrated_qc.queries[0]
diff --git a/tests/unit_tests/async_events/async_query_manager_tests.py b/tests/unit_tests/async_events/async_query_manager_tests.py
index b4ae06dfc3..85ea114201 100644
--- a/tests/unit_tests/async_events/async_query_manager_tests.py
+++ b/tests/unit_tests/async_events/async_query_manager_tests.py
@@ -14,12 +14,14 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from unittest import mock
+from unittest.mock import ANY, Mock
-from unittest.mock import Mock
-
+from flask import g
from jwt import encode
from pytest import fixture, raises
+from superset import security_manager
from superset.async_events.async_query_manager import (
AsyncQueryManager,
AsyncQueryTokenException,
@@ -38,6 +40,12 @@ def async_query_manager():
return query_manager
+def set_current_as_guest_user():
+ g.user = security_manager.get_guest_user_from_token(
+ {"user": {}, "resources": [{"type": "dashboard", "id": "some-uuid"}]}
+ )
+
+
def test_parse_channel_id_from_request(async_query_manager):
encoded_token = encode(
{"channel": "test_channel_id"}, JWT_TOKEN_SECRET, algorithm="HS256"
@@ -65,3 +73,70 @@ def test_parse_channel_id_from_request_bad_jwt(async_query_manager):
with raises(AsyncQueryTokenException):
async_query_manager.parse_channel_id_from_request(request)
+
+
+@mock.patch("superset.is_feature_enabled")
+def test_submit_chart_data_job_as_guest_user(
+ is_feature_enabled_mock, async_query_manager
+):
+ is_feature_enabled_mock.return_value = True
+ set_current_as_guest_user()
+ job_mock = Mock()
+ async_query_manager._load_chart_data_into_cache_job = job_mock
+ job_meta = async_query_manager.submit_chart_data_job(
+ channel_id="test_channel_id",
+ form_data={},
+ )
+
+ job_mock.delay.assert_called_once_with(
+ {
+ "channel_id": "test_channel_id",
+ "errors": [],
+ "guest_token": {
+ "resources": [{"id": "some-uuid", "type": "dashboard"}],
+ "user": {},
+ },
+ "job_id": ANY,
+ "result_url": None,
+ "status": "pending",
+ "user_id": None,
+ },
+ {},
+ )
+
+ assert "guest_token" not in job_meta
+
+
+@mock.patch("superset.is_feature_enabled")
+def test_submit_explore_json_job_as_guest_user(
+ is_feature_enabled_mock, async_query_manager
+):
+ is_feature_enabled_mock.return_value = True
+ set_current_as_guest_user()
+ job_mock = Mock()
+ async_query_manager._load_explore_json_into_cache_job = job_mock
+ job_meta = async_query_manager.submit_explore_json_job(
+ channel_id="test_channel_id",
+ form_data={},
+ response_type="json",
+ )
+
+ job_mock.delay.assert_called_once_with(
+ {
+ "channel_id": "test_channel_id",
+ "errors": [],
+ "guest_token": {
+ "resources": [{"id": "some-uuid", "type": "dashboard"}],
+ "user": {},
+ },
+ "job_id": ANY,
+ "result_url": None,
+ "status": "pending",
+ "user_id": None,
+ },
+ {},
+ "json",
+ False,
+ )
+
+ assert "guest_token" not in job_meta
(superset) 05/16: fix(security manager): Users should not have access to all draft dashboards (#27015)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5a6109b2dee046764babcfef4a5a67c32f2256ea
Author: Vitor Avila <96...@users.noreply.github.com>
AuthorDate: Wed Feb 7 13:20:41 2024 -0300
fix(security manager): Users should not have access to all draft dashboards (#27015)
(cherry picked from commit 01e2f8ace31950ca337a6a8d7348d37c59cf8126)
---
superset/security/manager.py | 31 ++++++++++++----------
.../dashboards/security/security_rbac_tests.py | 24 ++++++++++-------
2 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 28046d0fef..e11ab68e74 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -2018,25 +2018,28 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
if self.is_admin() or self.is_owner(dashboard):
return
- # RBAC and legacy (datasource inferred) access controls.
+ # TODO: Once a better sharing flow is in place, we should move the
+ # dashboard.published check here so that it's applied to both
+ # regular RBAC and DASHBOARD_RBAC
+
+ # DASHBOARD_RBAC logic - Manage dashboard access through roles.
+ # Only applicable in case the dashboard has roles set.
if is_feature_enabled("DASHBOARD_RBAC") and dashboard.roles:
if dashboard.published and {role.id for role in dashboard.roles} & {
role.id for role in self.get_user_roles()
}:
return
- elif (
- # To understand why we rely on status and give access to draft dashboards
- # without roles take a look at:
- #
- # - https://github.com/apache/superset/pull/24350#discussion_r1225061550
- # - https://github.com/apache/superset/pull/17511#issuecomment-975870169
- #
- not dashboard.published
- or not dashboard.datasources
- or any(
- self.can_access_datasource(datasource)
- for datasource in dashboard.datasources
- )
+
+ # REGULAR RBAC logic
+ # User can only acess the dashboard in case:
+ # It doesn't have any datasets; OR
+ # They have access to at least one dataset used.
+ # We currently don't check if the dashboard is published,
+ # to allow creators to share a WIP dashboard with a viewer
+ # to collect feedback.
+ elif not dashboard.datasources or any(
+ self.can_access_datasource(datasource)
+ for datasource in dashboard.datasources
):
return
diff --git a/tests/integration_tests/dashboards/security/security_rbac_tests.py b/tests/integration_tests/dashboards/security/security_rbac_tests.py
index 792c9d1716..4f1cc5b221 100644
--- a/tests/integration_tests/dashboards/security/security_rbac_tests.py
+++ b/tests/integration_tests/dashboards/security/security_rbac_tests.py
@@ -404,22 +404,28 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
for dash in published_dashboards + draft_dashboards:
revoke_access_to_dashboard(dash, "Public")
- def test_get_draft_dashboard_without_roles_by_uuid(self):
+ def test_cannot_get_draft_dashboard_without_roles_by_uuid(self):
"""
Dashboard API: Test get draft dashboard without roles by uuid
"""
admin = self.get_user("admin")
- dashboard = self.insert_dashboard("title", "slug1", [admin.id])
- assert not dashboard.published
- assert dashboard.roles == []
+
+ database = create_database_to_db(name="test_db_rbac")
+ table = create_datasource_table_to_db(
+ name="test_datasource_rbac", db_id=database.id, owners=[admin]
+ )
+ dashboard_to_access = create_dashboard_to_db(
+ dashboard_title="test_dashboard_rbac",
+ owners=[admin],
+ slices=[create_slice_to_db(datasource_id=table.id)],
+ )
+ assert not dashboard_to_access.published
+ assert dashboard_to_access.roles == []
self.login(username="gamma")
- uri = f"api/v1/dashboard/{dashboard.uuid}"
+ uri = f"api/v1/dashboard/{dashboard_to_access.uuid}"
rv = self.client.get(uri)
- assert rv.status_code == 200
- # rollback changes
- db.session.delete(dashboard)
- db.session.commit()
+ assert rv.status_code == 403
def test_cannot_get_draft_dashboard_with_roles_by_uuid(self):
"""
(superset) 13/16: build(deps): bump csstype from 2.6.9 to 3.1.3 in /superset-frontend (#26716)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4704380b2a0c05001f414489345f5aecd384b2cf
Author: dependabot[bot] <49...@users.noreply.github.com>
AuthorDate: Tue Jan 23 20:25:01 2024 -0700
build(deps): bump csstype from 2.6.9 to 3.1.3 in /superset-frontend (#26716)
Signed-off-by: dependabot[bot] <su...@github.com>
Co-authored-by: dependabot[bot] <49...@users.noreply.github.com>
---
superset-frontend/package-lock.json | 18 ++++++++++++++----
.../packages/superset-ui-core/package.json | 2 +-
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
index e308f2aed6..6cacdb3a0b 100644
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "superset",
- "version": "0.0.0-dev",
+ "version": "3.1.0",
"lockfileVersion": 2,
"requires": true,
"packages": {
"": {
"name": "superset",
- "version": "0.0.0-dev",
+ "version": "3.1.0",
"license": "Apache-2.0",
"workspaces": [
"packages/*",
@@ -62018,7 +62018,7 @@
"@types/rison": "0.0.6",
"@types/seedrandom": "^2.4.28",
"@vx/responsive": "^0.0.199",
- "csstype": "^2.6.4",
+ "csstype": "^3.1.3",
"d3-format": "^1.3.2",
"d3-interpolate": "^1.4.0",
"d3-scale": "^3.0.0",
@@ -62112,6 +62112,11 @@
"resolved": "https://registry.npmjs.org/@types/d3-time/-/d3-time-3.0.0.tgz",
"integrity": "sha512-sZLCdHvBUcNby1cB6Fd3ZBrABbjz3v1Vm90nysCQ6Vt7vd6e/h9Lt7SiJUoEX0l4Dzc7P5llKyhqSi1ycSf1Hg=="
},
+ "packages/superset-ui-core/node_modules/csstype": {
+ "version": "3.1.3",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz",
+ "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw=="
+ },
"packages/superset-ui-core/node_modules/d3-array": {
"version": "2.12.1",
"resolved": "https://registry.npmjs.org/d3-array/-/d3-array-2.12.1.tgz",
@@ -77236,7 +77241,7 @@
"@types/rison": "0.0.6",
"@types/seedrandom": "^2.4.28",
"@vx/responsive": "^0.0.199",
- "csstype": "^2.6.4",
+ "csstype": "^3.1.3",
"d3-format": "^1.3.2",
"d3-interpolate": "^1.4.0",
"d3-scale": "^3.0.0",
@@ -77291,6 +77296,11 @@
"resolved": "https://registry.npmjs.org/@types/d3-time/-/d3-time-3.0.0.tgz",
"integrity": "sha512-sZLCdHvBUcNby1cB6Fd3ZBrABbjz3v1Vm90nysCQ6Vt7vd6e/h9Lt7SiJUoEX0l4Dzc7P5llKyhqSi1ycSf1Hg=="
},
+ "csstype": {
+ "version": "3.1.3",
+ "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.3.tgz",
+ "integrity": "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw=="
+ },
"d3-array": {
"version": "2.12.1",
"resolved": "https://registry.npmjs.org/d3-array/-/d3-array-2.12.1.tgz",
diff --git a/superset-frontend/packages/superset-ui-core/package.json b/superset-frontend/packages/superset-ui-core/package.json
index 62deb7709b..91d7d28a32 100644
--- a/superset-frontend/packages/superset-ui-core/package.json
+++ b/superset-frontend/packages/superset-ui-core/package.json
@@ -41,7 +41,7 @@
"@types/rison": "0.0.6",
"@types/seedrandom": "^2.4.28",
"@vx/responsive": "^0.0.199",
- "csstype": "^2.6.4",
+ "csstype": "^3.1.3",
"d3-format": "^1.3.2",
"d3-interpolate": "^1.4.0",
"d3-scale": "^3.0.0",
(superset) 15/16: fix: bump FAB to 4.3.11 (#27039)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 534e8f394e96667a04a9f31e3fa2865b0786b2ce
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Wed Feb 7 15:58:19 2024 +0000
fix: bump FAB to 4.3.11 (#27039)
---
requirements/base.txt | 9 ++++-----
setup.py | 2 +-
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/requirements/base.txt b/requirements/base.txt
index da95b8c571..95eeaa450c 100644
--- a/requirements/base.txt
+++ b/requirements/base.txt
@@ -88,7 +88,7 @@ dnspython==2.1.0
# via email-validator
email-validator==1.1.3
# via flask-appbuilder
-exceptiongroup==1.1.1
+exceptiongroup==1.2.0
# via cattrs
flask==2.2.5
# via
@@ -104,7 +104,7 @@ flask==2.2.5
# flask-session
# flask-sqlalchemy
# flask-wtf
-flask-appbuilder==4.3.10
+flask-appbuilder==4.3.11
# via apache-superset
flask-babel==1.0.0
# via flask-appbuilder
@@ -141,9 +141,7 @@ geographiclib==1.52
geopy==2.2.0
# via apache-superset
greenlet==2.0.2
- # via
- # shillelagh
- # sqlalchemy
+ # via shillelagh
gunicorn==21.2.0
# via apache-superset
hashids==1.3.1
@@ -345,6 +343,7 @@ typing-extensions==4.4.0
# apache-superset
# cattrs
# flask-limiter
+ # kombu
# limits
# shillelagh
tzdata==2023.3
diff --git a/setup.py b/setup.py
index 4307b51c87..423a21d887 100644
--- a/setup.py
+++ b/setup.py
@@ -83,7 +83,7 @@ setup(
"cryptography>=41.0.2, <41.1.0",
"deprecation>=2.1.0, <2.2.0",
"flask>=2.2.5, <3.0.0",
- "flask-appbuilder>=4.3.10, <5.0.0",
+ "flask-appbuilder>=4.3.11, <5.0.0",
"flask-caching>=2.1.0, <3",
"flask-compress>=1.13, <2.0",
"flask-talisman>=1.0.0, <2.0",
(superset) 11/16: fix(drill): no rows returned (#27073)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit f440a6a5355a12ce92d66bf308e3888b54e004b9
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Mon Feb 12 12:11:06 2024 -0500
fix(drill): no rows returned (#27073)
(cherry picked from commit 0950bb7b7dd4658a112cc90e2d813267836ae002)
---
setup.py | 2 +-
superset/db_engine_specs/drill.py | 37 ++++++++++++++++++++++++++++++-------
2 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/setup.py b/setup.py
index 996d6dfe47..4307b51c87 100644
--- a/setup.py
+++ b/setup.py
@@ -154,7 +154,7 @@ setup(
],
"db2": ["ibm-db-sa>0.3.8, <=0.4.0"],
"dremio": ["sqlalchemy-dremio>=1.1.5, <1.3"],
- "drill": ["sqlalchemy-drill==0.1.dev"],
+ "drill": ["sqlalchemy-drill>=1.1.4, <2"],
"druid": ["pydruid>=0.6.5,<0.7"],
"duckdb": ["duckdb-engine>=0.9.5, <0.10"],
"dynamodb": ["pydynamodb>=0.4.2"],
diff --git a/superset/db_engine_specs/drill.py b/superset/db_engine_specs/drill.py
index fb42409b4e..276ff5b185 100644
--- a/superset/db_engine_specs/drill.py
+++ b/superset/db_engine_specs/drill.py
@@ -14,8 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
+from __future__ import annotations
+
from datetime import datetime
-from typing import Any, Optional
+from typing import Any
from urllib import parse
from sqlalchemy import types
@@ -60,8 +63,8 @@ class DrillEngineSpec(BaseEngineSpec):
@classmethod
def convert_dttm(
- cls, target_type: str, dttm: datetime, db_extra: Optional[dict[str, Any]] = None
- ) -> Optional[str]:
+ cls, target_type: str, dttm: datetime, db_extra: dict[str, Any] | None = None
+ ) -> str | None:
sqla_type = cls.get_sqla_column_type(target_type)
if isinstance(sqla_type, types.Date):
@@ -76,8 +79,8 @@ class DrillEngineSpec(BaseEngineSpec):
cls,
uri: URL,
connect_args: dict[str, Any],
- catalog: Optional[str] = None,
- schema: Optional[str] = None,
+ catalog: str | None = None,
+ schema: str | None = None,
) -> tuple[URL, dict[str, Any]]:
if schema:
uri = uri.set(database=parse.quote(schema.replace(".", "/"), safe=""))
@@ -89,7 +92,7 @@ class DrillEngineSpec(BaseEngineSpec):
cls,
sqlalchemy_uri: URL,
connect_args: dict[str, Any],
- ) -> Optional[str]:
+ ) -> str | None:
"""
Return the configured schema.
"""
@@ -97,7 +100,7 @@ class DrillEngineSpec(BaseEngineSpec):
@classmethod
def get_url_for_impersonation(
- cls, url: URL, impersonate_user: bool, username: Optional[str]
+ cls, url: URL, impersonate_user: bool, username: str | None
) -> URL:
"""
Return a modified URL with the username set.
@@ -117,3 +120,23 @@ class DrillEngineSpec(BaseEngineSpec):
)
return url
+
+ @classmethod
+ def fetch_data(
+ cls,
+ cursor: Any,
+ limit: int | None = None,
+ ) -> list[tuple[Any, ...]]:
+ """
+ Custom `fetch_data` for Drill.
+
+ When no rows are returned, Drill raises a `RuntimeError` with the message
+ "generator raised StopIteration". This method catches the exception and
+ returns an empty list instead.
+ """
+ try:
+ return super().fetch_data(cursor, limit)
+ except RuntimeError as ex:
+ if str(ex) == "generator raised StopIteration":
+ return []
+ raise