Conditional Requests / HTTP Caching

[BGR360]

The GitHub API supports conditional requests per HTTP RFC 7234. From the GitHub API docs (emphasis my own):

A big part of being a good API citizen is respecting rate limits by caching information that hasn't changed. The API supports conditional requests and helps you do the right thing.
[...]
The 304 status indicates that the resource hasn't changed since the last time we asked for it and the response will contain no body. As a bonus, 304 responses don't count against your rate limit.

For those that have ambitious goals for their GitHub-integrated projects, this is very valuable, as the 5,000-requests-per-hour rate limit may become very limiting without caching.

Another crate, octorust, provides support for HTTP caching as an optional feature. I was planning to use octorust for a project for this reason, but I discovered Octocrab and I would prefer to use the higher-level constructs provided by it.

I think the path to supporting this may not be too hard. There is a reqwest-middleware-cache crate that appears to provide a very easy shim layer around a reqwest::Client.

Here's what I imagine the usage would look like:

Cargo.toml:

[dependencies]
octocrab = { version = "^0.15", features = ["http_cache"] }

main.rs:

let octocrab = octocrab::Octocrab::builder()
    .http_cache("~/.cache/github")
    .build()?;

[XAMPPRocky]

Thank you for your issue! While I would definitely like to have this feature in octocrab, I'm hesitant to add any reqwest middleware as I want to remove reqwest as a dependency in this library (see #168), and adding middleware would make this harder.

Also I would not want to just have filesystem based caching, ideally it would be a pluggable solution (such as a trait) that allows people to provide their own caching implementation, because a lot of people aren't having long running processes and don't want to fill the filesystem with cache requests when we're also not providing cleanup.

So to summarise, I'm in favour of having it, but want to move to Sans-IO first (or use an agnostic approach), and I want a cache storage trait that handles the actual caching problem.

[iwahbe]

Has there been any progress on this issue? If not, I'd be interested in contributing. I don't want to duplicate existing work though.

[XAMPPRocky]

@iwahbe Thank you for your interest! There hasn't been any work on this as far as I know. Before implementing this though, we need to implement #99 which is also open to contributions.

[srid]

Looks like this is ready to be implemented. However, before that happens, what is the best way to go about providing caching at the application level (without changing this library yet, that is)?