Skip to content

Commit 228f3bc

Browse files
eschuthosfirke
authored andcommitted
feat: add chart id and dataset id to global logs (apache#26443)
1 parent 335c10b commit 228f3bc

File tree

2 files changed

+70
-4
lines changed

2 files changed

+70
-4
lines changed

superset/charts/data/api.py

+20-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,13 @@
4747
from superset.exceptions import QueryObjectValidationError
4848
from superset.extensions import event_logger
4949
from superset.models.sql_lab import Query
50-
from superset.utils.core import create_zip, get_user_id, json_int_dttm_ser
50+
from superset.utils.core import (
51+
create_zip,
52+
DatasourceType,
53+
get_user_id,
54+
json_int_dttm_ser,
55+
)
56+
from superset.utils.decorators import logs_context
5157
from superset.views.base import CsvResponse, generate_download_headers, XlsxResponse
5258
from superset.views.base_api import statsd_metrics
5359

@@ -421,6 +427,19 @@ def _get_data_response(
421427
def _load_query_context_form_from_cache(self, cache_key: str) -> dict[str, Any]:
422428
return QueryContextCacheLoader.load(cache_key)
423429

430+
def _map_form_data_datasource_to_dataset_id(
431+
self, form_data: dict[str, Any]
432+
) -> dict[str, Any]:
433+
return {
434+
"dataset_id": form_data.get("datasource", {}).get("id")
435+
if isinstance(form_data.get("datasource"), dict)
436+
and form_data.get("datasource", {}).get("type")
437+
== DatasourceType.TABLE.value
438+
else None,
439+
"slice_id": form_data.get("form_data", {}).get("slice_id"),
440+
}
441+
442+
@logs_context(context_func=_map_form_data_datasource_to_dataset_id)
424443
def _create_query_context_from_form(
425444
self, form_data: dict[str, Any]
426445
) -> QueryContext:

tests/integration_tests/charts/data/api_tests.py

+50-3
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
from flask import Response
2929
from tests.integration_tests.conftest import with_feature_flags
30+
from superset.charts.data.api import ChartDataRestApi
3031
from superset.models.sql_lab import Query
3132
from tests.integration_tests.base_tests import SupersetTestCase, test_client
3233
from tests.integration_tests.annotation_layers.fixtures import create_annotation_layers
@@ -47,7 +48,6 @@
4748
from superset.errors import SupersetErrorType
4849
from superset.extensions import async_query_manager_factory, db
4950
from superset.models.annotations import AnnotationLayer
50-
from superset.models.slice import Slice
5151
from superset.superset_typing import AdhocColumn
5252
from superset.utils.core import (
5353
AnnotationType,
@@ -88,7 +88,9 @@ def setUp(self) -> None:
8888
BaseTestChartDataApi.query_context_payload_template = get_query_context(
8989
"birth_names"
9090
)
91-
self.query_context_payload = copy.deepcopy(self.query_context_payload_template)
91+
self.query_context_payload = (
92+
copy.deepcopy(self.query_context_payload_template) or {}
93+
)
9294

9395
def get_expected_row_count(self, client_id: str) -> int:
9496
start_date = datetime.now()
@@ -124,7 +126,49 @@ def quote_name(self, name: str):
124126
@pytest.mark.chart_data_flow
125127
class TestPostChartDataApi(BaseTestChartDataApi):
126128
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
127-
def test_with_valid_qc__data_is_returned(self):
129+
def test__map_form_data_datasource_to_dataset_id(self):
130+
# arrange
131+
self.query_context_payload["datasource"] = {"id": 1, "type": "table"}
132+
# act
133+
response = ChartDataRestApi._map_form_data_datasource_to_dataset_id(
134+
ChartDataRestApi, self.query_context_payload
135+
)
136+
# assert
137+
assert response == {"dataset_id": 1, "slice_id": None}
138+
139+
# takes malformed content without raising an error
140+
self.query_context_payload["datasource"] = "1__table"
141+
# act
142+
response = ChartDataRestApi._map_form_data_datasource_to_dataset_id(
143+
ChartDataRestApi, self.query_context_payload
144+
)
145+
# assert
146+
assert response == {"dataset_id": None, "slice_id": None}
147+
148+
# takes a slice id
149+
self.query_context_payload["datasource"] = None
150+
self.query_context_payload["form_data"] = {"slice_id": 1}
151+
# act
152+
response = ChartDataRestApi._map_form_data_datasource_to_dataset_id(
153+
ChartDataRestApi, self.query_context_payload
154+
)
155+
# assert
156+
assert response == {"dataset_id": None, "slice_id": 1}
157+
158+
# takes missing slice id
159+
self.query_context_payload["datasource"] = None
160+
self.query_context_payload["form_data"] = {"foo": 1}
161+
# act
162+
response = ChartDataRestApi._map_form_data_datasource_to_dataset_id(
163+
ChartDataRestApi, self.query_context_payload
164+
)
165+
# assert
166+
assert response == {"dataset_id": None, "slice_id": None}
167+
168+
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
169+
@mock.patch("superset.utils.decorators.g")
170+
def test_with_valid_qc__data_is_returned(self, mock_g):
171+
mock_g.logs_context = {}
128172
# arrange
129173
expected_row_count = self.get_expected_row_count("client_id_1")
130174
# act
@@ -133,6 +177,9 @@ def test_with_valid_qc__data_is_returned(self):
133177
assert rv.status_code == 200
134178
self.assert_row_count(rv, expected_row_count)
135179

180+
# check that global logs decorator is capturing from form_data
181+
assert isinstance(mock_g.logs_context.get("dataset_id"), int)
182+
136183
@staticmethod
137184
def assert_row_count(rv: Response, expected_row_count: int):
138185
assert rv.json["result"][0]["rowcount"] == expected_row_count

0 commit comments

Comments
 (0)