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

feat: Introduce asset discovery response caching behind flag #419

Merged
merged 9 commits into from
Nov 8, 2019

Conversation

Robdel12
Copy link
Contributor

@Robdel12 Robdel12 commented Nov 8, 2019

What is this?

Sometimes the Percy Agent asset discovery service can overload a customers test server, which means sometimes snapshots are missing assets. Right now, we hit their applications test server for each asset at each width.

Off by default, this PR introduces a way to enable response caching for asset discovery. This is so our asset discovery service only will hit their applications test server for a given asset once (if we get a 200 response). In theory, it should reduce the number of SDK related missing assets (most commonly seen in rails apps)

@Robdel12 Robdel12 requested a review from wwilsman November 8, 2019 22:03
test/utils/response-cache.test.ts Outdated Show resolved Hide resolved
@Robdel12 Robdel12 requested a review from wwilsman November 8, 2019 22:17
Copy link
Contributor

@wwilsman wwilsman left a comment

Choose a reason for hiding this comment

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

🎮

@Robdel12 Robdel12 merged commit 7d7dd52 into master Nov 8, 2019
@delete-merged-branch delete-merged-branch bot deleted the rd/cache-responses branch November 8, 2019 22:31
djones pushed a commit that referenced this pull request Nov 8, 2019
# [0.20.0](v0.19.7...v0.20.0) (2019-11-08)

### Features

* Introduce asset discovery response caching behind flag ([#419](#419)) ([7d7dd52](7d7dd52))
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.

2 participants