Caching: handling 304 responses

[gr2m]

This is a follow up for https://github.com/octokit/node-github/pull/673#issuecomment-353894916 /cc @jsg2021

The documentation on Conditional requests says

Most responses return an ETag header. Many responses also return a Last-Modified header. You can use the values of these headers to make subsequent requests to those resources using the If-None-Match and If-Modified-Since headers, respectively. If the resource has not changed, the server will return a 304 Not Modified.

Maybe more importantly for us, it says

Note: Making a conditional request and receiving a 304 response does not count against your Rate Limit, so we encourage you to use it whenever possible.

(this should probably be added to the documentation on Best practices for integrators?)

My question is: do you / should we handle caching as part of the Octokit libraries?

I would say yes, the same way there should be APIs for pagination or handling of rate limits. I’m curious what we currently do in the other implementations

[jsg2021]

If you add transparent handling of caching, you should not preclude consumers opting out and handling it themselves... or provide a cache-provider plug-in api (so consumers can serialize/persist the cache in whatever form they want... be it memcache, json files, etc)

[gr2m]

My current idea is to make an official plugin that people can use, but can replace with their own. But I’m curious to hear what maintainers of octokit.net & octokit.rb have worked out

[M-Zuber]

Not a maintainer but I found some links related to octokit.net that might be useful

• A PR to implement caching based on ETags
• Caching plugin for Octokit.net
• Unfinished conversation about baking cache support into Octokit.net

[ryangribble]

Here is another octokit.net discussion about my thoughts on implementing ETag/caching support.

https://github.com/octokit/octokit.net/issues/1636#issuecomment-318802919

From my perspective I prefer the idea of exposing the necessary information to consumers (eg a way for them to know what the ETag of a response was, and a way to provide that ETag or a timestamp on a call) and let them use them as they see fit, as opposed to "magic" internal workings that automatically cache responses and use ETags internally without the user knowing about it etc.

I also prefer the idea of returning a blank collection when the 304 response is received, rather than returning a cached response - again because I feel like octokit is an API wrapper and not an actual application layer itself

[kytrinyx]

💯 to providing helper methods to make it easy to do pagination, caching, and handling rate limit responses, etc. I'd rather not do it transparently, for all the reasons @ryangribble mentions.

[Disturbing]

Would love to see this feature implemented! Exposing this in ApiOptions alone should be the way. Agreed that it should be an Api Wrapper, not an Application Layer.

Common features that sit on top of OctoKit.Net such as adding extension methods that cache this stuff in appointed repos is how I'd see this going.