ktmud commented on a change in pull request #10963:
URL:
https://github.com/apache/incubator-superset/pull/10963#discussion_r493791375
##########
File path: superset/utils/decorators.py
##########
@@ -72,6 +80,12 @@ def wrapper(*args: Any, **kwargs: Any) -> ETagResponseMixin:
if request.method == "POST":
return f(*args, **kwargs)
+ # if it is dashboard request but feature is not eabled,
+ # do not use cache
+ is_dashboard = is_dashboard_request(kwargs)
Review comment:
Instead of adding a helper function, maybe we can just expand
`dashboard_id_or_slug` in `wrapper` and make this code more transparent?
```python
def wrapper(*args: Any, dashboard_id_or_slug: str=None, **kwargs: Any) ->
ETagResponseMixin:
# ...
is_dashboard = dashboard_id_or_slug is not None
```
Better yet, we should probably aim for keeping all dashboard specific logics
out of `etag_cache` so it stays generic. Maybe add a `skip=` parameter that
runs the feature flag check.
```python
@etag_cache(skip=lambda: is_feature_enabled("ENABLE_DASHBOARD_ETAG_HEADER"))
```
```python
def etag_cache(
max_age: int,
check_perms: Callable[..., Any],
skip: Optional[Callable[..., Any]] = None,
) -> Callable[..., Any]:
def decorator(f: Callable[..., Any]) -> Callable[..., Any]:
def wrapper(*args: Any, **kwargs: Any) -> ETagResponseMixin:
check_perms(*args, **kwargs)
if request.method == "POST" or (skip and skip(*args, **kwargs)):
return f(*args, **kwargs)
```
##########
File path: superset/utils/decorators.py
##########
@@ -89,13 +103,25 @@ def wrapper(*args: Any, **kwargs: Any) ->
ETagResponseMixin:
raise
logger.exception("Exception possibly due to cache
backend.")
+ # if cache is stale?
+ if check_latest_changed_on:
+ latest_changed_on = check_latest_changed_on(*args, **kwargs)
+ if response and response.last_modified:
+ latest_record = response.last_modified.replace(
+ tzinfo=timezone.utc
+ ).astimezone(tz=None)
+ if latest_changed_on.timestamp() >
latest_record.timestamp():
+ response = None
Review comment:
Can probably rename `check_latest_changed_on ` to
`get_latest_changed_on` and do
```python
if get_latest_changed_on:
latest_changed_on = get_latest_changed_on(*args, **kwargs)
if response and response.last_modified and response.last_modified <
latest_changed_on:
response = None
else:
latest_changed_on = datetime.utcnow()
```
----------------------------------------------------------------
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]
