eclipse-csi / otterdog

OtterDog is a tool to manage GitHub organizations at scale using a configuration as code approach. It is actively used by the Eclipse Foundation to manage its numerous projects hosted on GitHub.
https://otterdog.readthedocs.org
Eclipse Public License 2.0
23 stars 3 forks source link

Using ETag for conditional requests to GitHub Rest API is not stable wrt the used token #201

Closed netomi closed 1 week ago

netomi commented 7 months ago

When using conditional requests to the GitHub API, and the token that will be used changes (installation tokens need to be recreated every hour), the existing cached reposonse can not be reused as GitHub will return 200 instead of 304.

This seems to be a known issue and limitation of using ETags in many cases, see https://stackoverflow.com/questions/28060116/which-is-more-reliable-for-github-api-conditional-requests-etag-or-last-modifie

Unfortunately, upon inspection a Last-Modified-Since headers is also not always available in the requests we usually do, so right now everytime the token needs to be refreshed, the existing cached responses will need to be refreshed as well.

Need to reach out to GitHub support if this is intended and likely to change in the future.

netomi commented 6 months ago

Confirmation from GitHub that this actually not supported right now but may be in the future:

Greetings from GitHub.

It's currently expected for ETags to be auth token specific. To workaround this, some of our customers use the If-Modified-Since; however, as you correctly noted, not all endpoints are supported yet. App tokens are time-limited for security and there's no way to modify that. While there isn't a solution to this at present, we'e added your name to an internal customer feedback issue where we're tracking interest in ETags that persist across multiple users, which would cover this case (same user, different tokens) as well.

We can't promise if or when we'll add this, but we recommend following the GitHub Changelog for future developments. We understand this is cold comfort as a presently impacted customer and wish we had better news to share, but we're afraid we don't at this time.

Using If-Modified-Since only could help, but would require some changes in the aiohttp-client-cache library which we use for conditional requests.

netomi commented 1 week ago

Its a bit annoying but not really a problem, in the worst case we would make a bit more requests as required.