wpt.fyi icon indicating copy to clipboard operation
wpt.fyi copied to clipboard

Use searchcache results if summary file is old format

Open DanielRyanSmith opened this issue 3 years ago • 1 comments

This change:

  • adds a check before summary files are used to determine if the summaries are in the new format (they have the "_v2" suffix in their bucket URL).
  • relegates any queries that involve old summary files to the search cache, and will be processed at runtime if needed.
  • updates mock summary data used in test files to use the new summary format.

Decision process for handling queries: Summary File Decision

An additional PR will clean up some logic previously added to handle both new and old summary formats together.

DanielRyanSmith avatar Jul 28 '22 23:07 DanielRyanSmith

Thanks for taking a look James! I was hoping to view the changes deployed to a staging environment to see how they interact with a functional searchcache component. I usually add useful commenting and context for review when things feel more finalized, so I hope this wasn't too much of a mess to understand. 🙂

DanielRyanSmith avatar Jul 29 '22 16:07 DanielRyanSmith

Thanks for taking a look @jcscottiii and @KyleJu 😊 Sorry I took so long to respond!

Overall it would be great if you could break down changes based on different services - one for searchcache and one for processor. (I understand the change to processor is rather minimal in this case). It easier to test/deploy/revert one service at a time.

I did a bit of this

On the design side, it seems to me that we have to validate new summary files for both webapp and searchcache? Does it mean that we have to direct all the traffic to searchcache first?

The summary files are not validated on webapp, but the summary URL is sliced and used to determine the URL for getting single test data results, because the single test data is prefaced with the same name as the summary (I've updated the method in test-file-results.js with a comment description to better explain what's happening there, as it wasn't obvious). For any test run query outside of single test views, the back end is sent a request and determines how to process the query.

When determining isSimpleQ, there is some additional work having to look at the summary file URLs and see if they have the correct suffix. However, this is relatively negligible and the possible strain comes from having to disregard the old summary files and aggregate the test results at query time, which is something that would need to happen even if we tried to determine the summary file viability on the webapp. (I hope this makes sense - it's a convoluted explanation 😅 )

DanielRyanSmith avatar Aug 19 '22 22:08 DanielRyanSmith

@jcscottiii @KyleJu This PR is ready for review once again. I've tried to thoroughly explain the changes file-by-file in the description above. Hopefully that makes it a bit easier to understand the approach. Let me know if there's anything I can do to make this easier!

DanielRyanSmith avatar Sep 06 '22 16:09 DanielRyanSmith

Thanks for the error handling help @jcscottiii 😊 maybe looks better now.

DanielRyanSmith avatar Sep 07 '22 17:09 DanielRyanSmith

Thanks for all the help @jcscottiii @KyleJu 😊

DanielRyanSmith avatar Sep 07 '22 20:09 DanielRyanSmith