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



##########
File path: superset/common/query_context.py
##########
@@ -201,7 +232,18 @@ def cache_timeout(self) -> int:
             return self.datasource.database.cache_timeout
         return config["CACHE_DEFAULT_TIMEOUT"]
 
-    def cache_key(self, query_obj: QueryObject, **kwargs: Any) -> 
Optional[str]:
+    def cache_key(self, **extra: Any) -> str:
+        """
+        The cache key is made out of the key/values from self.cached_values, 
plus any
+        other key/values in `extra`
+        """
+        key_prefix = "qc-"
+        cache_dict = self.cache_values.copy()
+        cache_dict.update(extra)
+
+        return generate_cache_key(cache_dict, key_prefix)

Review comment:
       It seems that we're calculating a hash on the `cache_values` dict, which 
contains the raw individual query dicts. For the same reason that we don't 
calculate the cache key on the raw query dict but instead call 
`QueryObject.cache_key()`, I believe we need to call the same method when 
calculating the cache key for `QueryContext`. So something like this:
   ```python
   self.cache_values = {
      ...
      "queries": [QueryObject(**query_obj).cache_key() for query_obj in 
self.queries],
      ...
   }
   ```
   This ensures the `QueryContext` cache key is always in sync with its 
`QueryObject`s.




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to