You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2022/09/13 13:56:40 UTC

[superset] branch master updated: fix(cache): respect default cache timeout on v1 chart data requests (#21441)

This is an automated email from the ASF dual-hosted git repository.

villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 05b97fff4d fix(cache): respect default cache timeout on v1 chart data requests (#21441)
05b97fff4d is described below

commit 05b97fff4dd56a480405b4ada65de712b3028ecc
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Tue Sep 13 15:56:04 2022 +0200

    fix(cache): respect default cache timeout on v1 chart data requests (#21441)
---
 .github/workflows/superset-python-unittest.yml   |  2 +-
 scripts/python_tests.sh                          |  2 +-
 superset/charts/schemas.py                       |  8 ++-
 superset/common/query_context_processor.py       |  6 +++
 tests/integration_tests/charts/data/api_tests.py | 63 +++++++++++++++++++++++-
 5 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/superset-python-unittest.yml b/.github/workflows/superset-python-unittest.yml
index 64db4d3e64..1ff07375d4 100644
--- a/.github/workflows/superset-python-unittest.yml
+++ b/.github/workflows/superset-python-unittest.yml
@@ -37,7 +37,7 @@ jobs:
           python-version: ${{ matrix.python-version }}
           cache: 'pip'
           cache-dependency-path: 'requirements/testing.txt'
-# TODO: separated requiermentes.txt file just for unit tests
+# TODO: separated requirements.txt file just for unit tests
       - name: Install dependencies
         if: steps.check.outcome == 'failure'
         uses: ./.github/actions/cached-dependencies
diff --git a/scripts/python_tests.sh b/scripts/python_tests.sh
index 0554f8ca65..6491a3f6f9 100755
--- a/scripts/python_tests.sh
+++ b/scripts/python_tests.sh
@@ -32,4 +32,4 @@ superset init
 
 echo "Running tests"
 
-pytest --durations-min=2 --maxfail=1 --cov-report= --cov=superset "$@"
+pytest --durations-min=2 --maxfail=1 --cov-report= --cov=superset ./tests/integration_tests "$@"
diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py
index c06768d127..ffd64e8bb0 100644
--- a/superset/charts/schemas.py
+++ b/superset/charts/schemas.py
@@ -1194,6 +1194,12 @@ class ChartDataQueryContextSchema(Schema):
     query_context_factory: Optional[QueryContextFactory] = None
     datasource = fields.Nested(ChartDataDatasourceSchema)
     queries = fields.List(fields.Nested(ChartDataQueryObjectSchema))
+    custom_cache_timeout = fields.Integer(
+        description="Override the default cache timeout",
+        required=False,
+        allow_none=True,
+    )
+
     force = fields.Boolean(
         description="Should the queries be forced to load from the source. "
         "Default: `false`",
@@ -1255,7 +1261,7 @@ class ChartDataResponseResult(Schema):
     )
     cache_timeout = fields.Integer(
         description="Cache timeout in following order: custom timeout, datasource "
-        "timeout, default config timeout.",
+        "timeout, cache default timeout, config default cache timeout.",
         required=True,
         allow_none=True,
     )
diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py
index b253caa6b9..ae3202981b 100644
--- a/superset/common/query_context_processor.py
+++ b/superset/common/query_context_processor.py
@@ -434,6 +434,12 @@ class QueryContextProcessor:
         cache_timeout_rv = self._query_context.get_cache_timeout()
         if cache_timeout_rv:
             return cache_timeout_rv
+        if (
+            data_cache_timeout := config["DATA_CACHE_CONFIG"].get(
+                "CACHE_DEFAULT_TIMEOUT"
+            )
+        ) is not None:
+            return data_cache_timeout
         return config["CACHE_DEFAULT_TIMEOUT"]
 
     def cache_key(self, **extra: Any) -> str:
diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py
index 67b9826d26..73d33cd793 100644
--- a/tests/integration_tests/charts/data/api_tests.py
+++ b/tests/integration_tests/charts/data/api_tests.py
@@ -21,7 +21,7 @@ import unittest
 import copy
 from datetime import datetime
 from io import BytesIO
-from typing import Optional
+from typing import Any, Dict, Optional
 from unittest import mock
 from zipfile import ZipFile
 
@@ -915,3 +915,64 @@ class TestGetChartDataApi(BaseTestChartDataApi):
         unique_genders = {row["male_or_female"] for row in data}
         assert unique_genders == {"male", "female"}
         assert result["applied_filters"] == [{"column": "male_or_female"}]
+
+
+@pytest.fixture()
+def physical_query_context(physical_dataset) -> Dict[str, Any]:
+    return {
+        "datasource": {
+            "type": physical_dataset.type,
+            "id": physical_dataset.id,
+        },
+        "queries": [
+            {
+                "columns": ["col1"],
+                "metrics": ["count"],
+                "orderby": [["col1", True]],
+            }
+        ],
+        "result_type": ChartDataResultType.FULL,
+        "force": True,
+    }
+
+
+@mock.patch(
+    "superset.common.query_context_processor.config",
+    {
+        **app.config,
+        "CACHE_DEFAULT_TIMEOUT": 1234,
+        "DATA_CACHE_CONFIG": {
+            **app.config["DATA_CACHE_CONFIG"],
+            "CACHE_DEFAULT_TIMEOUT": None,
+        },
+    },
+)
+def test_cache_default_timeout(test_client, login_as_admin, physical_query_context):
+    rv = test_client.post(CHART_DATA_URI, json=physical_query_context)
+    assert rv.json["result"][0]["cache_timeout"] == 1234
+
+
+def test_custom_cache_timeout(test_client, login_as_admin, physical_query_context):
+    physical_query_context["custom_cache_timeout"] = 5678
+    rv = test_client.post(CHART_DATA_URI, json=physical_query_context)
+    assert rv.json["result"][0]["cache_timeout"] == 5678
+
+
+@mock.patch(
+    "superset.common.query_context_processor.config",
+    {
+        **app.config,
+        "CACHE_DEFAULT_TIMEOUT": 100000,
+        "DATA_CACHE_CONFIG": {
+            **app.config["DATA_CACHE_CONFIG"],
+            "CACHE_DEFAULT_TIMEOUT": 3456,
+        },
+    },
+)
+def test_data_cache_default_timeout(
+    test_client,
+    login_as_admin,
+    physical_query_context,
+):
+    rv = test_client.post(CHART_DATA_URI, json=physical_query_context)
+    assert rv.json["result"][0]["cache_timeout"] == 3456