[Place Page Revamp] Fix one chart and add API Tests #4922
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…arallelize expensive operations (datacommonsorg#4912) With some of the recent place page work, the latency on the api calls has reached levels above what we should. This PR adds parallelization to our expensive operations with the related_places, and chats endpoints for the revamped place page. This also refactors the code significantly to reuse code across different parts of the functions, and create more granular helper functions. In a follow up PR I will add thorough testing on all these helpers and the main api endpoints. See Latency with this PR applied: https://paste.googleplex.com/4715161129320448 See latency without this PR: https://paste.googleplex.com/5451439923789824 Essentially, this leads to an approximately 40-50% speed up in the charts endpoint, and 30-40% for the related places endpoint.
gmechali
changed the title
dwnoble
approved these changes
Feb 5, 2025
dwnoble
reviewed
Feb 5, 2025
| @@ -442,6 +443,17 @@ async def fetch_and_process_stats(): | |||
| return valid_chart_configs | |||
|
|
|||
|
|
|||
| @cache.memoize(timeout=TIMEOUT) | |||
| async def memoized_filter_chart_config_for_data_existence( | |||
There was a problem hiding this comment.
ah that was my bad. this is supposed to be called from the API since we still want the memoization to happen. I moved this into a separate method because the cache.memoize annotation was causing some issues with the tests starting up since we don't have the cache initialization in this. I tried messing with it but couldn't figure it out. I can try again, but having the memoized method, and the implemetnation separate worked fine. just a little code duplication.
But the function there is now pretty well tested.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes one chart metadata to remove scaling on units that don't need it.
Adds significant test coverage across our utils for the new place page. Creates helpers for mocking data. More testing to come.