-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
browser cache #841
browser cache #841
Conversation
7d4e9be
to
fae69c1
Compare
I'm confident this will increase the perceived performance for users re-visiting the application. 👍 I think we also want to give our endpoints the possibility to both define how long a cache is valid before a new response should be fetched ( The way I understand how this flag can be used: E.g. for the (slow) well trajectory endpoint, we could have a |
fe2ee3f
to
519defd
Compare
backend_py/primary/primary/config.py
Outdated
print(f"{RESOURCE_SCOPES_DICT=}") | ||
|
||
DEFAULT_CACHE_MAX_AGE = 3600 # 1 hour | ||
DEFAULT_STALE_WHILE_REVALIDATE = 3600 # 1 hour |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just thinking loud, since most of the data we request/aggregate on is quite static (e.g. calculations performed on a completed FMU run): Should we have the defaults to be react-query-staleTime <= react-query-cacheTime << DEFAULT_CACHE_MAX_AGE << DEFAULT_STALE_WHILE_REVALIDATA
?
The "stale while revalidate" increases the time window where we have a browser cache hit (= where we immediately show data). At the same time, the user will not be stuck with "old" data in cache longer than max-age (pending a first request after max-age
, and a subsequent React component re-render/re-mount and timeout of React Query staleTime
). It would be interesting to see how this dynamic and potential upside turns out in practice, if the default is to have stale-while-revalidate (significantly) larger than max-age.
af4708e
to
5c70469
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! This will be interesting to see the effect of in practice! ⏩
|
||
from . import schemas | ||
|
||
router = APIRouter() | ||
|
||
|
||
@router.get("/fields") | ||
@no_cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably could be cached. It very rarely changes - and when it does the user has to wait for access to sync through infrastructure and Azure anyway. E.g. max-age
of one minute and a default stale-while-revalidate
of one day?
max-age
andstale-while-revalidate
time.@no_cache
and@add_custom_cache_time(max_age:int, stale_while_revalidate:int)