search

zooniverse / json-api-client

Apache License 2.0
10 stars 5 forks source link

[WIP] use superagent cached get / head req’s #31

Closed camallen closed 6 years ago

camallen commented 8 years ago

DO NOT MERGE

Looking to cut down the number of API req's and came across this module to auto cache GET / HEAD req's with superagent. I tried it out in PFE and it seems to work properly but I'm a little concerned about cache expiration timing especially on certain request routes like subjects, etc. Any thoughts on this?

superagent-cache docs https://github.com/jpodwys/superagent-cache#basic-usage

CacheModule docs https://github.com/jpodwys/cache-service-cache-module

brian-c commented 8 years ago

Strongly against this. It's a patch to work around poorly written components. I'd actually like to remove the caching behavior from this lib entirely once our components are smarter about fetching data.

camallen commented 8 years ago

@brian-c i get it - i'm trying to reduce the number of API calls from PFE, for instance on the classify page the workflows and tutorials (mini course) resources are requested on every new subjects even though they have not changed... basically just spamming the API

camallen commented 8 years ago

The more i think this, the more I think it's generally a good idea. Browsers do this by default for resources via response cache headers (rely on the server)...web servers have internal cache layers, server side data access layers have internal cache's, etc. Why remove / reject the client side data access layer cache? Yes caching is hard i completely agree and maybe that is the reason but we've got one now and it doesn't work with query params. This should fix it but replaces a dependency to a superagent lib...

I think we should seriously consider leaving the data cache layer in and expanding it to cache all requests.

camallen commented 8 years ago

I'm going to have a look at caching the API responses and invert the problem.

camallen commented 8 years ago

I'm a little worried about stale data and unintended side effects but would appreciated thoughts about this from @zooniverse/developers

eatyourgreens commented 7 years ago

Rebased to latest master.

camallen commented 7 years ago

@eatyourgreens noting that this library is only tested with superagent 1.x.

simoneduca commented 7 years ago

After some research, https://github.com/petercrona/ladda seems quite a promising caching solution that would work with superagent.