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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
