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

Add support for If-Modified-Since conditional requests #374

Open
Hixie opened this issue Jun 14, 2023 · 2 comments
Open

Add support for If-Modified-Since conditional requests #374

Hixie opened this issue Jun 14, 2023 · 2 comments

Comments

@Hixie
Copy link
Contributor

Hixie commented Jun 14, 2023

GitHub supports a mechanism by which if nothing has changed since the last time a request was made, it will save you the rate limits: https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#conditional-requests

I'm interested in implementing this feature. I'm curious if you have opinions about how it should be implemented.

From an API perspective what seems like the simplest option is to add a field to some of the model classes (e.g. Issue) that exposes the Last-Modified header of the request that obtained the data (e.g. Issue.httpLastModified), and have some of the methods that call into the GitHub API take an optional named argument (e.g. ifModifiedSince) that is used to configure the request, and someone would pass in the httpLastModified value from the last time they did the request as the value of the ifModifiedSince parameter.

However, it's not entirely clear to me what the best implementation strategy is within the package itself. The current architecture doesn't have any way to pass out-of-band data like HTTP headers.

Alternatively we could do something similar to the rate limit headers, and store the last request's Last-Modified header on the GitHub object. That seems easy to implement, if not quite as clean from an API perspective.

@github-actions
Copy link

👋 Thanks for reporting! @robrbecker will take a look.

@Hixie
Copy link
Contributor Author

Hixie commented Jun 16, 2023

Looks like _applyExpandos may be relevant to this.

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

No branches or pull requests

1 participant