mistercrunch commented on a change in pull request #7032: Fetch charts with GET
to benefit from browser cache and conditional requests
URL:
https://github.com/apache/incubator-superset/pull/7032#discussion_r288411786
##########
File path: superset/utils/decorators.py
##########
@@ -29,3 +41,61 @@ def stats_timing(stats_key, stats_logger):
raise e
finally:
stats_logger.timing(stats_key, now_as_float() - start_ts)
+
+
+def etag_cache(max_age, check_perms=bool):
+ """
+ A decorator for caching views and handling etag conditional requests.
+
+ The decorator caches the response, returning headers for etag and last
+ modified. If the client makes a request that matches, the server will
+ return a "304 Not Mofified" status.
+
+ If no cache is set, the decorator will still set the ETag header, and
+ handle conditional requests.
+
+ """
+ def decorator(f):
+ @wraps(f)
+ def wrapper(*args, **kwargs):
+ # check if the user can access the resource
+ check_perms(*args, **kwargs)
Review comment:
I was digging around to try and figure out where the datasource access
permission is done nowadays, and found it here in the etag_cache decorator. I
feel like it's not the right place for it.
I understand that this needs to happen prior to reading from cache, but
maybe it should be done as a prior decorator, or maybe both of these routines
should be done inside a method instead of decorators, to avoid calling get_viz
twice.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
