[Discover] Replace RxJS firstValueFrom with RxJS lastValueFrom to prevent problem with partial results #134682
Conversation
kertal
added
Feature:Discover
| * helper function to build a query body for Elasticsearch | ||
| * https://www.elastic.co/guide/en/elasticsearch/reference/current//query-dsl-ids-query.html | ||
| */ | ||
| export function buildSearchBody( |
There was a problem hiding this comment.
just a little code style change, moving the helper function to the bottom, so when scrolling the doc, the important function is displayed first
kertal
added
release_note:skip
| mockSearchResult.next({ | ||
| isPartial: true, | ||
| isRunning: false, | ||
| rawResponse: { | ||
| hits: { | ||
| hits: [], | ||
| }, | ||
| }, | ||
| }); | ||
| mockSearchResult.next({ | ||
| isPartial: false, | ||
| isRunning: false, | ||
| rawResponse: { | ||
| hits: { | ||
| hits: [record], | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
dear @Dosant , if you have minute, could you have a look at the unit tests, they work, but I would love to know, if I could make them better ❤️
There was a problem hiding this comment.
I think that will do it 👍 should capture the flow well.
Theoretically it is possible to make them behave more real-world by adding delay+fake timers between emits. Maybe even use rxjs-marbles for it.
But I think that's already good enough for the current use case
There was a problem hiding this comment.
thx @Dosant , for confirming 👍
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
kertal
changed the title
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @kertal |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added
the
backport missing
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
kibanamachine
removed
the
backport missing
…to prevent problem with partial results (#134682) (#134761) * [Discover] Replace RxJS firstValueFrom with RxJS lastValueFrom to prevent problem with partial results (#134682) (cherry picked from commit 69cd603) * Update es_query_expression.test.tsx Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>
Summary
The recent upgrade of RxJS #129087 led to refactoring of code. Some
.toPromise()was replaced withfirstValueFrom. This worked unless a partial result was returned before the final result. This PR replacesfirstValueFromwithlastValueFrom, to make sure, the final result is returned.Fixes #134138
Checklist