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

Adds caching to instant_articles_embed_oembed_html #775

Merged
merged 1 commit into from Oct 11, 2017

Conversation

Projects
None yet
3 participants
@emrikol
Copy link
Contributor

commented Oct 3, 2017

For each oEmbed on a page, an uncached remote request is made to check the oEmbed provider's URL. This patch adds a level of caching to stop from having to check each oEmbed on every page load.

It's unlikely that an oEmbed provider will change for a specific URL, so there shouldn't be any need to specify an expiration on the good cached data.

This PR:

  • Adds caching to instant_articles_embed_oembed_html

Fixes #685

Adds caching to instant_articles_embed_oembed_html
For each oEmbed on a page, an uncached remote request is made to check the oEmbed provider's URL.  This patch adds a level of caching to stop from having to check each oEmbed on every page load.

It's unlikely that an oEmbed provider will change for a specific URL, so there shouldn't be any need to specify an expiration on the good cached data.
@emrikol

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2017

Looks related to #337 too Nope, maybe not

@timjacobi timjacobi merged commit b6dd8b1 into master Oct 11, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@timjacobi

This comment has been minimized.

Copy link
Collaborator

commented Oct 11, 2017

LGTM, thanks for your contribution!

@sboisvert

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2017

Hi @timjacobi I was just wondering when the next version (that included this fix) was planned for (to see if I should patch our current versions or wait for a new version) Thanks!

@timjacobi

This comment has been minimized.

Copy link
Collaborator

commented Oct 19, 2017

We currently don’t have a release date so I recommend to patch for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.