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 cache buster to all GitHub API calls #449

Merged
merged 1 commit into from
Jun 12, 2017

Conversation

biilmann
Copy link
Contributor

This should solve issues like #308 and the issues with cached
API responses after deleting an entry

- Summary

GitHub's API does fairly aggressive caching for GET requests, at this causes problems in several cases. Especially when creating commit trees where a stale response from the GitHub API can make us try to base the changes on a wrong SHA and lead to issues like #308

This also caused problem for the work on deleting entries, since we would get a stale folder list right after entry deletion.

This PR takes the brute force approach of adding a cache buster timestamp as a query parameter to all GitHub API requests.

- Test plan

Warning - I haven't tested this yet. The old Ember prototype had a similar cache buster approach (though only on some requests) so I'm fairly confident in the solution. An alternative approach would be to only set the cache buster parameter for get requests that MUST be uncached.

- Description for the changelog

Fix stale reads from GitHub's API

- A picture of a cute animal (not mandatory but encouraged)

img_20170502_141429312

This should solve issues like #308 and the issues with cached
API responses after deleting an entry
Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Benaiah Benaiah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Benaiah Benaiah merged commit 546ca59 into master Jun 12, 2017
@tech4him1 tech4him1 deleted the add-cache-buster-to-github-api-calls branch August 26, 2017 15:43
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

Successfully merging this pull request may close these issues.

3 participants