Skip to content
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

a better caching experience for Octokit #530

Closed
shiftkey opened this issue Jul 10, 2014 · 13 comments
Closed

a better caching experience for Octokit #530

shiftkey opened this issue Jul 10, 2014 · 13 comments
Labels
Status: Stale Used by stalebot to clean house Type: Feature New feature or request

Comments

@shiftkey
Copy link
Member

After discussions in #369 about WinInet I'm leaning towards a different approach for a couple of reasons:

  • it's unclear if the WinInet issues will be fixed, let alone when
  • what about platforms where WinInet isn't available?
  • HttpClient has support for doing this "Russian Doll"-style of behaviour using DelegatingHandler

Tavis.PrivateCache has been proposed as a implementation of this, so I'm leaning towards one of two options around this:

Make Tavis.PrivateCache an development dependency, and bake it in

Pros:

  • easier for users to get benefits of it
  • less fiddling around with ctors

Cons:

  • harder to debug (perhaps make this opt-out for those who are feeling brave)

Expose DelegatingHandler in GitHubClient and let others do the work wiring it up

Pros:

  • more future-proof
  • others might want to do their own stuff with HttpClient

Cons:

  • harder to get benefits for new users
  • probably some more IoC pain

The work required is probably about the same, but I'm leaning toward option 1 because it'll be simpler for users of the library.

cc @paulcbetts @darrelmiller @niik

@haacked
Copy link
Contributor

haacked commented Jul 10, 2014

👍 to option 1

@darrelmiller
Copy link
Contributor

I am happy to help in any way I can. My recent blog post on the Vary header (http://bizcoder.com/the-insanity-of-the-vary-header) was my first step to fixing the current vary implementation and the next major task is do a persistent storage mechanism. The current implementation is only an in-memory store.

@shiftkey
Copy link
Member Author

The current implementation is only an in-memory store.

I think that's a "good enough" option for now for our use case, but perhaps down the track someone might want to implement their own backing store for it (imagine a service who wants to push that memory usage out of process, for example).

@nigel-sampson
Copy link
Contributor

I created a naive implementation of the latter solution by creating a custom http client https://github.com/nigel-sampson/octokit.caching

@anaisbetts
Copy link
Contributor

@shiftkey
Copy link
Member Author

@anaisbetts
Copy link
Contributor

Hubot img me Jonny Lee Miller 2015

@darrelmiller
Copy link
Contributor

Oh, where oh where are the other 48 hours I need in a day.

@shiftkey
Copy link
Member Author

Status update: I'm ranting and raving over in #781 about first class support for HttpClient/HttpMessageHandler - once I get that right I think this will be trivial to add - and I already have crazy ideas in my head about how to do it...

@darrelmiller
Copy link
Contributor

Yesterday I did a major update to my HTTPCache to fix the problems with the way the vary header is handled. I also renamed it because how it can handle being a shared cache too! This also caused a change to the storage interface. Next step is to produce a persistent storage mechanism.

@shiftkey
Copy link
Member Author

@darrelmiller so if I head down the road towards something like this does that make your life easier or harder?

@darrelmiller
Copy link
Contributor

@shiftkey Much easier. Adding my cache would look pretty much exactly like you showed it in the example.

@github-actions
Copy link

github-actions bot commented Aug 3, 2022

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Stale label Aug 3, 2022
@nickfloyd nickfloyd added Type: Feature New feature or request Status: Stale Used by stalebot to clean house and removed category: feature labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Stale Used by stalebot to clean house Type: Feature New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants