-
Notifications
You must be signed in to change notification settings - Fork 891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bug] set saved search instance for Discover based on dataset #8689
[bug] set saved search instance for Discover based on dataset #8689
Conversation
Preventing any errors being thrown because of the index pattern not existing in the case of saved search was created with a dataset. Also fix issue that prevented saved search instance not being set even though loaded properly. The original fix that was removed fixed a previous issue but subsequent updates and refactors required an update that was uncaught. Issue: n/a Signed-off-by: Kawika Avilla <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8689 +/- ##
==========================================
- Coverage 60.86% 60.86% -0.01%
==========================================
Files 3793 3793
Lines 90456 90462 +6
Branches 14204 14207 +3
==========================================
- Hits 55059 55057 -2
- Misses 31908 31915 +7
- Partials 3489 3490 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a test here for the saved search flow, but can come in as a fast follow.
Signed-off-by: Kawika Avilla <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a test related to saved searches is failing?
http: services.http, | ||
data: services.data, | ||
}); | ||
pattern = await data.indexPatterns.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for my knowledge, this just reads from cache and doesn't make a request? If it does make a request, I'm a little confused by the sequence above (call, cache, call) - I'm assuming the cache call is actually what fetches given this usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question.
for anything that isn't an index pattern, it will read just from the cache. if nothing returns from the cache, then we will recreate the the dataset and save it as an index pattern when we call cache dataset. then we retrieve the actual index pattern we just created.
if it is an index pattern it will check the cache if it doesn't exist in the cache it will make it a request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving PR as test failure looks like test needs to be cleaned up, but can happen separately
test is fixed here opensearch-project/opensearch-dashboards-functional-test#1601 |
* [bug] set saved search instance for Discover based on dataset Preventing any errors being thrown because of the index pattern not existing in the case of saved search was created with a dataset. Also fix issue that prevented saved search instance not being set even though loaded properly. The original fix that was removed fixed a previous issue but subsequent updates and refactors required an update that was uncaught. Issue: n/a Signed-off-by: Kawika Avilla <[email protected]> * set saved search order Signed-off-by: Kawika Avilla <[email protected]> --------- Signed-off-by: Kawika Avilla <[email protected]> (cherry picked from commit e4281e4) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…#8692) * [bug] set saved search instance for Discover based on dataset Preventing any errors being thrown because of the index pattern not existing in the case of saved search was created with a dataset. Also fix issue that prevented saved search instance not being set even though loaded properly. The original fix that was removed fixed a previous issue but subsequent updates and refactors required an update that was uncaught. Issue: n/a * set saved search order --------- (cherry picked from commit e4281e4) Signed-off-by: Kawika Avilla <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…arch-project#8689) * [bug] set saved search instance for Discover based on dataset Preventing any errors being thrown because of the index pattern not existing in the case of saved search was created with a dataset. Also fix issue that prevented saved search instance not being set even though loaded properly. The original fix that was removed fixed a previous issue but subsequent updates and refactors required an update that was uncaught. Issue: n/a Signed-off-by: Kawika Avilla <[email protected]> * set saved search order Signed-off-by: Kawika Avilla <[email protected]> --------- Signed-off-by: Kawika Avilla <[email protected]>
Description
Preventing any errors being thrown because of the index pattern not existing in the case of saved search was created with a dataset.
Also fix issue that prevented saved search instance not being set even though loaded properly.
The original fix that was removed fixed a previous issue but subsequent updates and refactors required an update that was uncaught.
Issues Resolved
n/a
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration