web: use stream search results for export#45702
Conversation
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 3ba7222...65a311a.
|
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits 65a311a and 3ba7222 or learn more. Open explanation
|
| query={submittedURLQuery} | ||
| enableCodeInsights={codeInsightsEnabled && isCodeInsightsEnabled(props.settingsCascade)} | ||
| enableCodeMonitoring={enableCodeMonitoring} | ||
| resultsFound={resultsFound} |
There was a problem hiding this comment.
Lol we had the results here already haha 😶🌫️
|
@taras-yemets I think we should add a change log entry as well, wdyt? I think in general I’m not adding that often enough 😓 but especially for customer reported issues it would be good to include it if they ever need to find out which version they have to update to. |
100% agree with a change log entry here. Otherwise LGTM! Thanks for getting it done so quickly! |
| const [requestSearchResultsExport] = useExportSearchResultsQuery({ query, patternType, sourcegraphURL }) | ||
| const resultsFound = results ? results.results.length > 0 : false | ||
| const downloadResults = useCallback( | ||
| () => (results ? downloadSearchResults(results, sourcegraphURL, query) : undefined), | ||
| [results, sourcegraphURL, query] | ||
| ) |
There was a problem hiding this comment.
pinging a 3 month old PR, yay. But I think this is the root cause of a regression. There is a concept of display limit the streaming search API uses. Basically we only return 1500 results down to the browser (but continue streaming accurate result counts). Which means if you just use the results already downloaded to display you won't actually export all results. Here is were we hardcode that https://sourcegraph.com/github.com/sourcegraph/sourcegraph@db2922509bf00cd14fb30393a12d863974c15e88/-/blob/client/shared/src/search/stream.ts?L490
Here is the slack thread that just got started about this https://sourcegraph.slack.com/archives/CHEKCRWKV/p1678300443530819
Pinging here so hopefully one of the experts on this PR can turn around a fix :)
6 participants
Closes https://github.com/sourcegraph/sourcegraph/issues/45718
Uses stream search results for export.
Previously we were using two different approaches for search results page and for search results export.
The search results page uses stream search results and the search export called the GraphQL API (this approach migrated from sourcegraph-search-export extension). This caused inconsistencies in what users see on the results page and what results they download after clicking the 'Export results' button.
This PR:
SearchMatchinstead ofSearchResultFields): see updated results table headersTest plan
CI passes.
App preview:
Check out the client app preview documentation to learn more.