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

Conditionally fetch blog RSS feed #10015

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

astatide
Copy link

@astatide astatide commented Nov 8, 2024

Closes #2831

refactor! This wraps the existing _get_blog_feeds functionality via an enclosure that caches the result and date so a conditional get request can be performed.

Technical

This was done under an assumption that I probably should not change cache.memcache_memoize and leave it as is. This technically uses an alternate sort of 'cache' mechanism in that we're leaving the results inside the closure.

Also? It's been a hot minute since I've changed some python code, so help testing this PR to make sure it works would be really appreciated!

Testing

Screenshot

Stakeholders

Copy link
Collaborator

@jimchamp jimchamp left a comment

Choose a reason for hiding this comment

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

Thanks @astatide!
When testing this, I wasn't able to trigger a 304 response from the blog server. Were you able to?

Instead of all of these changes, can you instead modify the cache timeout to last for a day (as per this comment)?

Staff will be able to evict the cache entry for the blog posts if ever needed (Note for staff: Cache key for blog posts is upstream.get_blog_feeds$).

@jimchamp jimchamp added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conditionally fetch blog RSS feed - performance improvement
2 participants