Cache fulltext search results #10167
Conversation
jimchamp
changed the title
There was a problem hiding this comment.
The requirement was to cache the partially rendered HTML that replaces the loading indicator on "no results found" search pages. You've cached the JSON full-text search results.
Here is where the data is fetched for the partially rendered HTML. If those results are error-free, add the Cache-Control header. That is all that is needed.
|
Should I remove my previous changes and resubmit only the changes needed or should I keep my previous change and the change you suggested? |
|
|
||
| # Add caching headers only if there's no error | ||
| if 'error' not in results: | ||
| # Cache for 5 minutes (300 seconds) | ||
| web.header('Cache-Control', 'public, max-age=300') | ||
| web.header('Expires', web.http.expires(300)) | ||
| else: | ||
| # No caching for error responses | ||
| web.header('Cache-Control', 'no-store, no-cache, must-revalidate') | ||
| web.header('Pragma', 'no-cache') | ||
|
|
There was a problem hiding this comment.
| # Add caching headers only if there's no error | |
| if 'error' not in results: | |
| # Cache for 5 minutes (300 seconds) | |
| web.header('Cache-Control', 'public, max-age=300') | |
| web.header('Expires', web.http.expires(300)) | |
| else: | |
| # No caching for error responses | |
| web.header('Cache-Control', 'no-store, no-cache, must-revalidate') | |
| web.header('Pragma', 'no-cache') |
There was a problem hiding this comment.
Hi @jimchamp , I have done the added the above lines of code and removed my previous ones. It works as expected.
I guess you can merge this now.
There was a problem hiding this comment.
You'll have to push your changes before they can be reviewed and merged. Remove the Expires header before you do so.
Also add a note about how you tested this.
There was a problem hiding this comment.
Also, remove the not hits['hits'] part of the conditional.
There was a problem hiding this comment.
Hi @jimchamp, I have pushed my changes as you told. Please do check once and let me know if the results are as expected.
|
Remove the code that you've committed. Only submit what is necessary to close the linked issue. |
jimchamp
added
the
Needs: Submitter Input
Closes #10051
This PR achives reduction in time taken to show no results by about 0.5-0.8 s
Technical
The key differences are:
The modified code added explicit caching headers for successful responses
The modified code added specific no-cache headers for error responses
The modified code had more sophisticated header management
Testing
The above technical changes are done in the file in the path openlibrary\plugins\inside\code.py
The change is simple and just involved understanding the codebase and adding relevant codeline for caching.
It can be implemented by simply building the docker image via docker-compose build and then running it via docker-compose up.
Stakeholders
I have worked on this issue with @jimchamp as lead.