-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
dataviews.clearCache cleanup #136256
dataviews.clearCache cleanup #136256
Conversation
if (id) { | ||
this.dataViewCache.clear(id); | ||
} else { | ||
if (full) { |
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.
It seems this code had some undesirable functional overlap to begin with. the savedObjectsCache
is really a list of data views whereas the dataViewCache
actually stores DataView instances. They'd be cleared in slightly different situations, whether you're getting a fresh list or fresh DataView instance. What are the needs and intentions of the api consumer code? I think this gets lost when clearCache
always clears the data view list cache and optionally clears the dataView cache. Determining api consumers intentions might be more work than the code changes in which case perhaps we should deprecate this method and replace it with two new methods.
cfe91bb
to
01e579f
Compare
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
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.
LGTM!
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.
changes lgtm, nice work
Summary
dataviews.clearCache(id?: string)
was cleaned up, before it cleaned the data view saved objects cache as well as data view instance cache. The method was split into two methods:for apps that used to clear whole instance cache (discover) unified search data view selector was updated to refresh the field list on data view change and discover also refreshes the field list of a data view on load to make sure user always gets the recent field list.
Checklist
Delete any items that are not applicable to this PR.