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

Conditional Requests: etag vs. if-modified-since #1

Closed
andygrunwald opened this issue Mar 10, 2025 · 2 comments
Closed

Conditional Requests: etag vs. if-modified-since #1

andygrunwald opened this issue Mar 10, 2025 · 2 comments

Comments

@andygrunwald
Copy link

Thanks a lot for this research and the effort you put into it.
While most of the research is based on the etag, I was re-reading https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api?apiVersion=2022-11-28#use-conditional-requests-if-appropriate, it shows that you can either use the etag OR the if-modified-since header.

I was wondering:

This should work as expected as

  • you use standard caching
  • check against Github API only time based
  • avoid the Authorization to matter when the Vary header check comes in

What did I overlook?

@bored-engineer
Copy link
Owner

@andygrunwald That's a really really great point, that approach does seem to work well for API endpoints that return a Last-Modified header. However some endpoints do not, for example many of the "list" endpoints, ex:

$ gh api --verbose '/repos/bored-engineer/github-conditional-http-transport/issues' | grep -i 'Last-Modified'
$ gh api --verbose '/repos/bored-engineer/github-conditional-http-transport/issues/1' | grep -i 'Last-Modified'
< Last-Modified: Mon, 10 Mar 2025 20:56:30 GMT

@andygrunwald
Copy link
Author

Gotcha. This was new to me. Thanks for this hint.

I think this heavily depends on your use case and what your crawling behaviour looks like.
Calling the issues/ endpoint is returning you a list of issues but not the full object of each issue. At least you are able to cache the per-issue request. I would assume in many usecases this is the bigger chunk of used API requests.

But overall, I agree with you. Including the auth token into the etag with a short token time (install token) is a bit odd.

On another note: I raised this topic also in palantir/go-githubapp#437 as this app is build for such a usecase.

Thanks @bored-engineer.
I am closing this one now, as I see everything as clarified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants