-
Notifications
You must be signed in to change notification settings - Fork 27
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
SceneQueryRunner: decouple time range comparisons #587
Merged
Merged
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
bb51fca
SceneQueryRunner: decouple time range comparisons
sd2k 87eb1c6
Find first RequestAdder of each type, rather than stopping when first…
sd2k 9aa6700
Rename 'transform' to 'processor' and 'adder' to 'supplementer'
sd2k aea64d9
Return array from getClosestRequestAdders
sd2k c6aebd1
Rename SceneRequestSupplementer to SupplementalRequestProvider
sd2k 699f64e
Simplify extra request operator; add more comments
sd2k b0bd8a5
Add tests for supplementary request handling in query runner
sd2k 374bda5
s/supplemental/supplementary/g
sd2k 8d19b5d
Extract PreparedRequests type; document; use non-arrow function for p…
sd2k 5d867d5
Use map from provider constructor to object, rather than constructor …
sd2k 2b13e2a
Rename SupplementalRequestFoo to ExtraQueryFoo throughout
sd2k 61de2a1
Include annotations from secondary processed frames
sd2k c410bf2
s/ExtraQueryProcessor/ExtraQueryDataProcessor/
sd2k bdab9f4
Change ExtraQueryDataProcessor to return an Observable
sd2k 87ead7f
Add trivial processor to TestExtraQueryProvider
sd2k File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import { DataQueryRequest, PanelData } from "@grafana/data"; | ||
|
||
import { SceneObjectBase } from "../core/SceneObjectBase"; | ||
import { SceneObjectState } from "../core/types"; | ||
|
||
// A processor function called by the query runner with responses | ||
// to any extra requests. | ||
// | ||
// A processor function should accept two arguments: the data returned by the | ||
// _primary_ query, and the data returned by the `ExtraQueryProvider`'s | ||
// _secondary_ query. It should return a new `PanelData` representing the processed output. | ||
// It should _not_ modify the primary PanelData. | ||
// | ||
// Examples of valid processing include alignment of data between primary and secondary | ||
// (see the `timeShiftAlignmentProcessor` returned by `SceneTimeRangeCompare`), or doing | ||
// some more advanced processing such as fitting a time series model on the secondary data. | ||
// | ||
// See the docs for `extraQueryProcessingOperator` for more information. | ||
export type ExtraQueryDataProcessor = (primary: PanelData, secondary: PanelData) => PanelData; | ||
|
||
// An extra request that should be run by a query runner, and an optional | ||
// processor that should be called with the response data. | ||
export interface ExtraQueryDescriptor { | ||
// The extra request to add. | ||
req: DataQueryRequest; | ||
// An optional function used to process the data before passing it | ||
// to any transformations or visualizations. | ||
processor?: ExtraQueryDataProcessor; | ||
} | ||
|
||
// Indicates that this type wants to add extra requests, along with | ||
// optional processing functions, to a query runner. | ||
export interface ExtraQueryProvider<T extends SceneObjectState> extends SceneObjectBase<T> { | ||
// Get any extra requests and their required processors. | ||
getExtraQueries(request: DataQueryRequest): ExtraQueryDescriptor[]; | ||
// Determine whether a query should be rerun. | ||
// | ||
// When the provider's state changes this function will be passed both the previous and the | ||
// next state. The implementation can use this to determine whether the change should trigger | ||
// a rerun of the query or not. | ||
shouldRerun(prev: T, next: T): boolean; | ||
} | ||
|
||
export function isExtraQueryProvider(obj: any): obj is ExtraQueryProvider<any> { | ||
return typeof obj === 'object' && 'getExtraQueries' in obj; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Don't think it's suitable for this PR but if I want to offload my processing to a web worker this is going to need to return a Promise instead...
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.
If we don't introduce this now, it's going to be a breaking change in the future. I think it may be worth doing this straight away, wdyt @torkelo ?
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 was thinking we could change it to
export type ExtraQueryDataProcessor = (primary: PanelData, secondary: PanelData) => PanelData | Promise<PanelData>;
in future but it is clunky, I agree...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.
Can it be an observable (like the current post processing) that will be much easier to slot in to the rxjs pipeline
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.
yeah, +1 for that!
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've changed the type to
export type ExtraQueryDataProcessor = (primary: PanelData, secondary: PanelData) => Observable<PanelData>;
in bdab9f4, is that what you meant? 🤔 Think I got the types right but I might have misunderstood.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 that's exactly it @sd2k