Repository navigation
feat: [2312] Dashboard data store - Read from the store and remove the legacy datastore - #2316
Conversation
…ontext can't hold the store, instead of disabling it
…and unreachable-store warnings to ui-base
…it unchanged, and make the harness's warning checks real
n-lark
changed the title
cstns
left a comment
cstns
left a comment
There was a problem hiding this comment.
One more thing that's new with this PR: a flow that clears all global context (e.g. for (const k of global.keys()) global.set(k, undefined)) now wipes every widget's state, charts included, and it stays empty after a reload. The warning in the log explains it well. On main that pattern never touched the dashboard, so it's maybe worth mentioning in the docs or release notes.
| try { | ||
| getOrCreateStore(node.context().global)[TRIM_SERIES](node.id, trim) | ||
| } catch (err) { warnOnce(node, `a store write failed (${err.message}), so global.dashboardStore may be out of date`) } | ||
| store = attachToContext(globalContext, attachOptions()) |
There was a problem hiding this comment.
Noticed something with a persistent context store (localfilesystem, cache: true). The store only gets written to disk when something else in global context is set, and then only the values go. After a restart my text input came back with its old value, which main never did, and the chart read back as { payload: [points] } instead of a list of msgs (20 points for a "last 5 per topic" chart). The chart showed empty and recovered on the next message, but a widget calling get() would see the odd shape. So whether state survives a restart ends up depending on an unrelated global.set. Might be worth picking one behaviour, either always starting fresh like main or persisting properly.
There was a problem hiding this comment.
Yahh makes sense, went with starting fresh like main for now, which also fixes the chart coming back as { payload: [points] } here.
#2322 also keeps a chart's messages in step with its points for flow writes. I do want state to eventually survive restarts, but it can be settled in the persistence work #2237
| if (!store) { | ||
| datastore.disableStore() | ||
| node.warn('Dashboard data store disabled: the global context store doesn\'t hold objects by reference (e.g. cache: false), so live state can\'t work. Use the memory store or localfilesystem with cache: true.' + (unavailable ? ' Reading it failed with: ' + unavailable.message : '')) | ||
| node.warn('Dashboard data store: flows can\'t use global.dashboardStore, because the default context store doesn\'t keep values in memory (e.g. localfilesystem with cache: false). Dashboards still work normally. To use global.dashboardStore from flows, set contextStorage.default to memory, or localfilesystem with cache: true.' + (unavailable ? ' (Reason: ' + unavailable.message + ')' : '')) |
There was a problem hiding this comment.
Works nicely. I tried cache: false and the dashboard kept its data through a full redeploy. Minor: the warning shows on every full deploy, which might get a bit noisy for those users.
2 participants
Description
Last of three PRs moving reads onto the data store (#2239).
Widgets now read their saved data from the store, and the legacy datastore is removed. Widgets, including third-party ones, use the same API as before.
cache: false), the dashboard keeps its own copy in memory instead, so it still works.Known regression: charts with a time limit are slower than on
main, and page loads take about 2.5× as long. A follow-up PR will address this.Test plan
mainfor the same messages, including after browser interactions, a reload and removing widgetsRelated Issue(s)
Resolves #2312
Checklist
flowforge.yml?FlowFuse/helmto update ConfigMap TemplateFlowFuse/CloudProjectto update values for Staging/ProductionLabels
area:migrationlabel