[GitHub] [incubator-superset] john-bodley commented on a change in pull request #10061: fix: caching on viz with relative time ranges and time compare

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

john-bodley commented on a change in pull request #10061:
URL: https://github.com/apache/incubator-superset/pull/10061#discussion_r440615422



##########
File path: superset/viz.py
##########
@@ -396,8 +396,9 @@ def cache_key(self, query_obj: QueryObjectDict, **extra: Any) -> str:
         cache_dict = copy.copy(query_obj)
         cache_dict.update(extra)
 
-        for k in ["from_dttm", "to_dttm"]:
-            del cache_dict[k]
+        for k in ["from_dttm", "to_dttm", "inner_from_dttm", "inner_to_dttm"]:

Review comment:
       @etr2460 can you also make this change in `viz_sip38.py`? Also we should probably update the comment on line 392 to reflect the change.

##########
File path: tests/core_tests.py
##########
@@ -122,7 +122,16 @@ def test_viz_cache_key(self):
         cache_key = viz.cache_key(qobj)
 
         qobj["groupby"] = []
-        self.assertNotEqual(cache_key, viz.cache_key(qobj))
+        cache_key_with_groupby = viz.cache_key(qobj)
+        self.assertNotEqual(cache_key, cache_key_with_groupby)
+
+        self.assertNotEqual(
+            viz.cache_key(qobj), viz.cache_key(qobj, time_compare="12 weeks")
+        )
+
+        qobj["inner_from_dttm"] = "foo"

Review comment:
       Could we make this a `datetime` object for consistency?  I personally find unit tests can be a good mechanism for understanding how the code operates and thus ensuring the types are accurate helps to improve the readability. 




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