-
Notifications
You must be signed in to change notification settings - Fork 12
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
WIP: Streaming: Add streaming capability to certain query in twinmakers #74
Conversation
lean257
commented
Jun 30, 2022
- currently added to entity history
@@ -68,6 +69,11 @@ export class TwinMakerDataSource extends DataSourceWithBackend<TwinMakerQuery, T | |||
} | |||
|
|||
query(options: DataQueryRequest<TwinMakerQuery>): Observable<DataQueryResponse> { | |||
for (const target of options.targets) { | |||
if (target.queryType === TwinMakerQueryType.EntityHistory) { |
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.
should this be moved to the else statement on line 96 so that it starts streaming after the AWS API finishes scrolling through the results?
if (next.length) {
return next;
} else {
// start streaming
}
or is the idea to move scrolling to the backend for the queries that support streaming responses?
scope: LiveChannelScope.DataSource, | ||
namespace: ds.uid, | ||
path: `tail/${key}`, | ||
data: { |
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.
is this data also sent to RunStream
, or would that be handled somewhere else?
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.
@ryantxu answered this one in the daily. it sounds like this is also sent to RunStream
, which makes things much easier
res.Error = nil | ||
for i,_ := range res.Frames { | ||
res.Frames[i].SetMeta(&data.FrameMeta{ | ||
Channel: fmt.Sprintf("ds/%s/30s/%s/%s", ds.settings.UID, query.EntityId, query.ComponentName), |
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.
is this still needed with the new changes to datasource.ts
?
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.
ah no I'll clean up
"@grafana/toolkit": "8.2.0", | ||
"@grafana/e2e": "8.2.0", | ||
"@grafana/ui": "8.2.0", | ||
"@grafana/data": "8.5.0", |
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.
Any reason we're upgrading these?
We are not going to upgrade to grafana 8.5 so we cannot currently use streaming |