Skip to content
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

Merged
merged 4 commits into from
Sep 5, 2023
Merged

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Aug 31, 2023

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

@ryantxu ryantxu requested review from a team as code owners August 31, 2023 00:26
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.2.x milestone Aug 31, 2023
@ryantxu ryantxu requested a review from marefr August 31, 2023 00:26
@ryantxu ryantxu changed the title Plugins: Move filter back to Plugins: Move filter back to DataSourceWithBackend Aug 31, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2023

⚠️   Possible breaking changes

(Open the links below in a new tab to go to the correct steps)

grafana-data has possible breaking changes (more info)

Console output
Read our guideline

@github-actions github-actions bot added the levitate breaking change A label indicating a breaking change and assigned by Levitate. label Aug 31, 2023
Copy link
Member

@academo academo left a 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.

@academo academo requested review from academo and removed request for academo August 31, 2023 12:07
Comment on lines 270 to 278
/**
* Override to skip executing a query
*
* @returns false if the query should be skipped
*
* @virtual
*/
filterQuery?(query: TQuery): boolean;

Copy link
Contributor

@oshirohugo oshirohugo Aug 31, 2023

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.

@ryantxu ryantxu requested a review from academo August 31, 2023 16:49
@ryantxu ryantxu enabled auto-merge (squash) September 1, 2023 16:05
@ryantxu ryantxu merged commit f7522b6 into main Sep 5, 2023
17 of 18 checks passed
@ryantxu ryantxu deleted the move-filter branch September 5, 2023 22:34
chauchausoup pushed a commit to chauchausoup/grafana that referenced this pull request Sep 15, 2023
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/datasource area/frontend area/plugins levitate breaking change A label indicating a breaking change and assigned by Levitate. no-backport Skip backport of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugins: filterQuery isn't called for frontend data sources
4 participants