-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
Plugins: Move filter back to DataSourceWithBackend #74147
Conversation
(Open the links below in a new tab to go to the correct steps)
|
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.
Even if we put this here by mistake it is still a breaking change, specially if had been here for such a long time.
We should follow the deprecation process.
/** | ||
* Override to skip executing a query | ||
* | ||
* @returns false if the query should be skipped | ||
* | ||
* @virtual | ||
*/ | ||
filterQuery?(query: TQuery): boolean; | ||
|
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.
I think we can add a note in this documentation, stating that the filterQuery
won't work if the query
method from DataSourceWithBackend
is overwritten.
In the docs it says that DataSoruceWithBackend.query
is ideally final, but there are normal situations like streaming plugins where it needs to be overwritten.
In #41317 (two years ago! 😬). the filterQuery api was moved to the root datasource api level. However it is only executed within DataSourceWithBackend
In #74116 -- the proposal is to execute this in the panel query runner.
It is easy to argue that either approach is a breaking change -- executing somethign that was previously not executed, or a possible type error while developing. The 2nd seems safer to me
Who is this feature for?
Plugin developers
Which issue(s) does this PR fix?:
Fixes #47876