-
Notifications
You must be signed in to change notification settings - Fork 1
Add databrowser widget #130
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
Conversation
…atabrowser-widget
| }; | ||
| }); | ||
| }); | ||
| fetchedData = tmpData; |
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 fetchedData is overwritten on every iteration of the outer for loop. That does not matter if there is only one archiver, but for multiple archivers it's a bug.
I'm guessing that it should be: fetchedData = [...fetchedData, ...tmpData]
Can we guarantee the order of the data returned by the archivers? ie would the concatenated data from multiple data archives be in increasing date/time order or would we need to merge sort it?
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.
Good spot, thanks for that. My test files only had one archiver, so I missed this. The requests per archiver should all be for the same period of time, so it would be need to be merge sorted to combine with data from the other archivers. I'm not sure how often someone wants to plot data from different archivers on the same plot, but added this functionality just in case.
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.
In the interest of avoiding some scope creep, I think I would explicitly support one archiver, log a warning if there are more than one and add a ticket on the backlog to track a feature to support multiple archivers. If it turns out we need to support more than one archiver we can prioritise the ticket.
GregJHarris
left a comment
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.
Approved Thank you
| }; | ||
| }); | ||
| }); | ||
| fetchedData = tmpData; |
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.
In the interest of avoiding some scope creep, I think I would explicitly support one archiver, log a warning if there are more than one and add a ticket on the backlog to track a feature to support multiple archivers. If it turns out we need to support more than one archiver we can prioritise the ticket.
This PR adds a Databrowser widget, which builds on top of the existing StripChart widget and adds archived data.
Other changes made:
fetch. The Device and TabContainer components were also impacted by this and have been updateduseRefto prevent archived data being overwritten, also implementedbufferSizeandupdatePeriodproperties to limit size of datasets and how often data updates.