[GitHub] [incubator-superset] villebro commented on a change in pull request #12103: fix(async queries): Remove "force" param on cached data retrieval

[GitBox <gi...@apache.org>]

villebro commented on a change in pull request #12103:
URL: https://github.com/apache/incubator-superset/pull/12103#discussion_r546213738



##########
File path: superset/common/query_context.py
##########
@@ -60,8 +60,8 @@ class QueryContext:
     queries: List[QueryObject]
     force: bool
     custom_cache_timeout: Optional[int]
-    result_type: utils.ChartDataResultType
-    result_format: utils.ChartDataResultFormat
+    result_type: Union[utils.ChartDataResultType, str]
+    result_format: Union[utils.ChartDataResultFormat, str]

Review comment:
       This `Union` shouldn't be necessary here. Are you getting linting errors without it?

##########
File path: superset/common/query_context.py
##########
@@ -71,23 +71,22 @@ def __init__(  # pylint: disable=too-many-arguments
         queries: List[Dict[str, Any]],
         force: bool = False,
         custom_cache_timeout: Optional[int] = None,
-        result_type: Optional[utils.ChartDataResultType] = None,
-        result_format: Optional[utils.ChartDataResultFormat] = None,
+        result_type: Optional[Union[utils.ChartDataResultType, str]] = None,
+        result_format: Optional[Union[utils.ChartDataResultFormat, str]] = None,

Review comment:
       Same here.

##########
File path: tests/query_context_tests.py
##########
@@ -73,13 +74,41 @@ def test_schema_deserialization(self):
                 self.assertEqual(post_proc["operation"], payload_post_proc["operation"])
                 self.assertEqual(post_proc["options"], payload_post_proc["options"])
 
-    def test_cache_key_changes_when_datasource_is_updated(self):
+    def test_cache(self):
+        table_name = "birth_names"
+        table = self.get_table_by_name(table_name)
+        payload = get_query_context(table.name, table.id, table.type)
+        payload["force"] = True
+
+        query_context = ChartDataQueryContextSchema().load(payload)
+        query_object = query_context.queries[0]
+        query_cache_key = query_context.query_cache_key(query_object)
+
+        response = query_context.get_payload(cache_query_context=True)
+        cache_key = response["cache_key"]
+        assert cache_key
+
+        cached = cache_manager.cache.get(cache_key)
+        assert cached

Review comment:
       nit: should we `assert cached is not None` here to highlight that we're not expecting a truthy value but something that isn't `None`?

##########
File path: superset/common/query_context.py
##########
@@ -71,23 +71,22 @@ def __init__(  # pylint: disable=too-many-arguments
         queries: List[Dict[str, Any]],
         force: bool = False,
         custom_cache_timeout: Optional[int] = None,
-        result_type: Optional[utils.ChartDataResultType] = None,
-        result_format: Optional[utils.ChartDataResultFormat] = None,
+        result_type: Optional[Union[utils.ChartDataResultType, str]] = None,
+        result_format: Optional[Union[utils.ChartDataResultFormat, str]] = None,
     ) -> None:
         self.datasource = ConnectorRegistry.get_datasource(
             str(datasource["type"]), int(datasource["id"]), db.session
         )
         self.queries = [QueryObject(**query_obj) for query_obj in queries]
         self.force = force
         self.custom_cache_timeout = custom_cache_timeout
-        self.result_type = result_type or utils.ChartDataResultType.FULL
-        self.result_format = result_format or utils.ChartDataResultFormat.JSON
+        self.result_type = result_type or utils.ChartDataResultType.FULL.value
+        self.result_format = result_format or utils.ChartDataResultFormat.JSON.value

Review comment:
       And here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org