[GitHub] [incubator-superset] john-bodley commented on a change in pull request #8901: fix: add datasource.changed_on to cache_key

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

john-bodley commented on a change in pull request #8901: fix: add datasource.changed_on to cache_key
URL: https://github.com/apache/incubator-superset/pull/8901#discussion_r372541748
 
 

 ##########
 File path: superset/common/query_context.py
 ##########
 @@ -157,20 +157,25 @@ def cache_timeout(self) -> int:
             return self.datasource.database.cache_timeout
         return config["CACHE_DEFAULT_TIMEOUT"]
 
-    def get_df_payload(  # pylint: disable=too-many-locals,too-many-statements
-        self, query_obj: QueryObject, **kwargs
-    ) -> Dict[str, Any]:
-        """Handles caching around the df payload retrieval"""
+    def cache_key(self, query_obj: QueryObject, **kwargs) -> Optional[str]:
         extra_cache_keys = self.datasource.get_extra_cache_keys(query_obj.to_dict())
         cache_key = (
             query_obj.cache_key(
                 datasource=self.datasource.uid,
                 extra_cache_keys=extra_cache_keys,
+                changed_on=self.datasource.changed_on,
 
 Review comment:
   @villebro apologies for the late comment. In your PR description you mention that the cache key should be a function on whether the column or metric definitions associated with a datasource are changed. 
   
   On line #166 you merely use the datasource `change_on` and thus I was wondering whether a changes to the columns and/or metrics cascade, i.e., trigger an update to the datasource `changed_on`?

----------------------------------------------------------------
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


With regards,
Apache Git Services

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