From 9ee6398cb73b93d6d7b45140474a294900587ddf Mon Sep 17 00:00:00 2001 From: Mayur Date: Tue, 22 Nov 2022 16:30:54 +0530 Subject: [PATCH 1/7] fix --- superset/charts/data/api.py | 6 +++ .../charts/data/api_tests.py | 53 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py index c20fdde6fd94e..9254945bc3e7f 100644 --- a/superset/charts/data/api.py +++ b/superset/charts/data/api.py @@ -89,6 +89,11 @@ def get_data(self, pk: int) -> Response: description: The type in which the data should be returned schema: type: string + - in: query + name: force + description: Should the queries be forced to load from the source + schema: + type: boolean responses: 200: description: Query result @@ -130,6 +135,7 @@ def get_data(self, pk: int) -> Response: "format", ChartDataResultFormat.JSON ) json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL) + json_body["force"] = request.args.get("force", False) try: query_context = self._create_query_context_from_form(json_body) diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py index acf44be6f561e..9c2dbf4890d91 100644 --- a/tests/integration_tests/charts/data/api_tests.py +++ b/tests/integration_tests/charts/data/api_tests.py @@ -862,6 +862,59 @@ def test_chart_data_get(self): assert data["result"][0]["status"] == "success" assert data["result"][0]["rowcount"] == 2 + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_chart_data_get_forced(self): + """ + Chart data API: Test GET endpoint with force cache parameter + """ + chart = db.session.query(Slice).filter_by(slice_name="Genders").one() + chart.query_context = json.dumps( + { + "datasource": {"id": chart.table.id, "type": "table"}, + "force": False, + "queries": [ + { + "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00", + "granularity": "ds", + "filters": [], + "extras": { + "having": "", + "having_druid": [], + "where": "", + }, + "applied_time_extras": {}, + "columns": ["gender"], + "metrics": ["sum__num"], + "orderby": [["sum__num", False]], + "annotation_layers": [], + "row_limit": 50000, + "timeseries_limit": 0, + "order_desc": True, + "url_params": {}, + "custom_params": {}, + "custom_form_data": {}, + } + ], + "result_format": "json", + "result_type": "full", + } + ) + + # wrapping original class, so we can test output too. + with mock.patch( + "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand + ) as command: + rv = self.get_assert_metric( + f"api/v1/chart/{chart.id}/data/?force=true", "get_data" + ) + data = json.loads(rv.data.decode("utf-8")) + query_context = command.call_args.args[0] + + assert query_context.force == True + assert rv.mimetype == "application/json" + assert data["result"][0]["status"] == "success" + assert data["result"][0]["rowcount"] == 2 + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(GLOBAL_ASYNC_QUERIES=True) @mock.patch("superset.charts.data.api.QueryContextCacheLoader") From d317c58bb15fef5e2894cd33fcff2fcd53b7a960 Mon Sep 17 00:00:00 2001 From: Mayur Date: Sat, 26 Nov 2022 07:52:00 +0530 Subject: [PATCH 2/7] update reviews --- superset/charts/data/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py index 9254945bc3e7f..773229ad5f753 100644 --- a/superset/charts/data/api.py +++ b/superset/charts/data/api.py @@ -135,7 +135,7 @@ def get_data(self, pk: int) -> Response: "format", ChartDataResultFormat.JSON ) json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL) - json_body["force"] = request.args.get("force", False) + json_body["force"] = request.args.get("force") try: query_context = self._create_query_context_from_form(json_body) From 4b5fa45aa3585d5c28c96e6342b50c7978d9fe69 Mon Sep 17 00:00:00 2001 From: Mayur Date: Sat, 26 Nov 2022 08:13:11 +0530 Subject: [PATCH 3/7] revision 2 --- superset/charts/data/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py index 773229ad5f753..f47d6b543cdc7 100644 --- a/superset/charts/data/api.py +++ b/superset/charts/data/api.py @@ -135,7 +135,8 @@ def get_data(self, pk: int) -> Response: "format", ChartDataResultFormat.JSON ) json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL) - json_body["force"] = request.args.get("force") + if request.args.get("force"): + json_body["force"] = request.args.get("force") try: query_context = self._create_query_context_from_form(json_body) From 7d348c7b5602c7657c773cdaab595a09f81249cc Mon Sep 17 00:00:00 2001 From: Mayur Date: Sat, 26 Nov 2022 08:26:17 +0530 Subject: [PATCH 4/7] update marshmallow schema to have force as none --- superset/charts/data/api.py | 3 +-- superset/charts/schemas.py | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py index f47d6b543cdc7..773229ad5f753 100644 --- a/superset/charts/data/api.py +++ b/superset/charts/data/api.py @@ -135,8 +135,7 @@ def get_data(self, pk: int) -> Response: "format", ChartDataResultFormat.JSON ) json_body["result_type"] = request.args.get("type", ChartDataResultType.FULL) - if request.args.get("force"): - json_body["force"] = request.args.get("force") + json_body["force"] = request.args.get("force") try: query_context = self._create_query_context_from_form(json_body) diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index 34dd44b38c7aa..701d83ebe5866 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -1204,6 +1204,7 @@ class ChartDataQueryContextSchema(Schema): force = fields.Boolean( description="Should the queries be forced to load from the source. " "Default: `false`", + allow_none=True, ) result_type = EnumField(ChartDataResultType, by_value=True) From 33539e383841a2576dc64710c35eae74e027c7ae Mon Sep 17 00:00:00 2001 From: Mayur Date: Sat, 26 Nov 2022 09:09:27 +0530 Subject: [PATCH 5/7] update test to assert is_cached in output --- .../charts/data/api_tests.py | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py index 9c2dbf4890d91..2e52148abe7cc 100644 --- a/tests/integration_tests/charts/data/api_tests.py +++ b/tests/integration_tests/charts/data/api_tests.py @@ -901,19 +901,25 @@ def test_chart_data_get_forced(self): ) # wrapping original class, so we can test output too. - with mock.patch( - "superset.charts.data.api.ChartDataCommand", wraps=ChartDataCommand - ) as command: - rv = self.get_assert_metric( - f"api/v1/chart/{chart.id}/data/?force=true", "get_data" - ) - data = json.loads(rv.data.decode("utf-8")) - query_context = command.call_args.args[0] - assert query_context.force == True - assert rv.mimetype == "application/json" - assert data["result"][0]["status"] == "success" - assert data["result"][0]["rowcount"] == 2 + self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data") + + rv = self.get_assert_metric( + f"api/v1/chart/{chart.id}/data/?force=true", "get_data" + ) + + data = json.loads(rv.data.decode("utf-8")) + assert rv.json["result"][0]["is_cached"] is None + assert rv.mimetype == "application/json" + assert data["result"][0]["status"] == "success" + assert data["result"][0]["rowcount"] == 2 + + rv = self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data") + data = json.loads(rv.data.decode("utf-8")) + assert rv.json["result"][0]["is_cached"] is not None + assert rv.mimetype == "application/json" + assert data["result"][0]["status"] == "success" + assert data["result"][0]["rowcount"] == 2 @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(GLOBAL_ASYNC_QUERIES=True) From 0cb66fdbd0f724dfe1aaa86ea4f177389b8c7a5f Mon Sep 17 00:00:00 2001 From: Mayur Date: Sat, 26 Nov 2022 09:18:42 +0530 Subject: [PATCH 6/7] remove un-necessary comment --- tests/integration_tests/charts/data/api_tests.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py index 2e52148abe7cc..b2db75a197199 100644 --- a/tests/integration_tests/charts/data/api_tests.py +++ b/tests/integration_tests/charts/data/api_tests.py @@ -900,8 +900,6 @@ def test_chart_data_get_forced(self): } ) - # wrapping original class, so we can test output too. - self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data") rv = self.get_assert_metric( From e8802b907071925ffdab2f0bada031aafb86d61a Mon Sep 17 00:00:00 2001 From: Mayur Date: Sat, 26 Nov 2022 10:04:09 +0530 Subject: [PATCH 7/7] remove data asserts --- tests/integration_tests/charts/data/api_tests.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py index b2db75a197199..6904bc7df45cc 100644 --- a/tests/integration_tests/charts/data/api_tests.py +++ b/tests/integration_tests/charts/data/api_tests.py @@ -900,24 +900,17 @@ def test_chart_data_get_forced(self): } ) - self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data") + self.get_assert_metric(f"api/v1/chart/{chart.id}/data/?force=true", "get_data") + # should burst cache rv = self.get_assert_metric( f"api/v1/chart/{chart.id}/data/?force=true", "get_data" ) - - data = json.loads(rv.data.decode("utf-8")) assert rv.json["result"][0]["is_cached"] is None - assert rv.mimetype == "application/json" - assert data["result"][0]["status"] == "success" - assert data["result"][0]["rowcount"] == 2 + # should get response from the cache rv = self.get_assert_metric(f"api/v1/chart/{chart.id}/data/", "get_data") - data = json.loads(rv.data.decode("utf-8")) - assert rv.json["result"][0]["is_cached"] is not None - assert rv.mimetype == "application/json" - assert data["result"][0]["status"] == "success" - assert data["result"][0]["rowcount"] == 2 + assert rv.json["result"][0]["is_cached"] @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(GLOBAL_ASYNC_QUERIES=True)