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

Local (client) scope for dashboard nodes #430

Open
omrid01 opened this issue Dec 11, 2023 · 9 comments
Open

Local (client) scope for dashboard nodes #430

omrid01 opened this issue Dec 11, 2023 · 9 comments
Labels
feature-request New feature or request that needs to be turned into Epic/Story details needs-triage Needs looking at to decide what to do

Comments

@omrid01
Copy link

omrid01 commented Dec 11, 2023

Description

Dashboard nodes can be instantiated across multiple concurrent dashboard clients, but maintain a single state/value (replicated automatically across clients). In multi-user scenarios, there is a need for local client scope (for example: per-client input fields & progress indicators).
It is possible to provide this multi-user functionality by programming around the nodes (filtering messages based on client id), yet it would be beneficial to enable this as a built-in capability.
The proposed feature is to add a checkbox to all dashboard nodes (or at least to ui-template), indicating the scope. If checked, incoming messages will be tested for a client-id property, and will accept the message only if either:

  • The client id is matches the current client, or
  • The message has is no client id property

The flow will be able to dynamically set the message scope, by adding/deleting the client id property.

Note: the client id can be the existing socketid property, or a new dedicated id. It is assumed that all outgoing messages from dashboard nodes automatically include this id (as done today).

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

@omrid01 omrid01 added feature-request New feature or request that needs to be turned into Epic/Story details needs-triage Needs looking at to decide what to do labels Dec 11, 2023
@joepavitt
Copy link
Collaborator

Some complexities here to consider/discuss:

We currently use the concept of a datastore in our server-side architecture. Within this, when a msg is received by a node, we map this data against the node's ID into our datastore. When refreshing the dashboard we read the datastore and retrieve the latest message, loading our widget with the latest state.

We can restrict message comms to a client if a socketid is present in the msg, but how should this update our single-source of truth datastore? I see three primary options:

  1. Do not store any msg objects that contain a restriction (e.g. socketid) into our datastore.
    • This would mean, if Client A receives data that nobody else does, when they refresh, they'd return to the state all other clients see
    • We definitely cannot store a message into the datastore, if that msg was only sent to one client (but multiple are connected)
  2. Disable the datastore entirely if socketid is being included in msg events sent.
    • This would be no single source of truth is maintain by Dashboard 2.0
    • Instead, flows would need to load the relevant data (e.g. from a Database) each time a user loads a Dashboard
  3. Maintain a socketid mapping, in addition to Widget ID mapping, in our in-memory datastore.
    • Pro: Best UX for users
    • Con: Scales terribly, lots of rich data (e.g. charts) now stored for every user, rather than just once

Another factor to then consider is that, currently, we transmit msg.socketid on every event coming out of any Dashboard 2.0 nodes. If we implement the above, then, we need to have that socketid appended as an opt-in setting on the ui-base/ui-page or something along those lines.

@omrid01
Copy link
Author

omrid01 commented Dec 15, 2023

Hi Joe,
For nodes where the server acts as a centralized source of truth across all dashboard clients, The requirement to automatically populate all the nodes upon client connect makes sense. Yet storing & replaying the last received message does not guarantee this. It could be true for simple nodes (e.g. a text box showing the last received payload), but not for nodes populated by multiple messages, such as grids (where incoming messages add or remove rows), dynamically configured selection-lists, charts with historical trends etc.
It is unrealistic to save & replay a full message trail for each dashboard node every time a client connects. Moreover, some messages MUST NEVER be replayed. For example, messages coming from API, button etc., which invoke actions/flows.
My suggestion is to make this (replay last message) an optional user-setting in the node configuration.

Given the above limitations & complexity of restoring the last state of centralized ("server-based") nodes, IMHO it would be unrealistic to try store & restore client-specific nodes, and I would draw the line here and avoid it.
To summarize, each relevant dashboard node should have 2 checkboxes:

  • Store & replay the last incoming message upon client load
  • Local scope: if checked, the node will only accept messages with the same (or without) socket id, and will not receive any "Last message" replays

It should be fairly easy for the developer to initialize the local-scope nodes upon page load (e.g. via a template node sending a startup message at the beginning of its JavaScript section)

@bartbutenaers
Copy link
Contributor

I rather agree with the thoughts from @omrid01.
The state of a node will be build based on N input messages (like e.g. a floorplan svg drawing), so replay the last msg is most of the time not useful and indeed should be avoided.

This was the reason why I registered this issue some time ago: to let the input messages change the server side state in the datastore, instead of storing these messages themselves. So new clients get the latest state of the node, instead of the latest message. Like Flexdash used to do it.

@joepavitt
Copy link
Collaborator

but not for nodes populated by multiple messages, such as grids (where incoming messages add or remove rows), dynamically configured selection-lists, charts with historical trends etc.

Some important changes that I have built into Dashboard 2.0 to address this are:

  • Differentiate onLoad as a function, from onInput
    • This allows nodes to run different logic onLoad, such that it's not the equivalent of "replaying" an input per-say.
  • Datastore has both save (default) and append options
    • The default just saves the last msg received against the node. The append option however stores the history of msg, such that the history can be used onLoad to re-populate the content, e.g. ui-chart

Local scope: if checked, the node will only accept messages with the same (or without) socket id, and will not receive any "Last message" replays

I think I like this. Will be a mission to implement (needs to be duplicated across all core UI nodes), and would also then require all third-party widgets to implement this too though, which makes me hesitate.

What I'm tempted to do, is have this configuration live in the sidebar, that way, I can centralise the logic, and it will auto-handle any third-party widgets too?

So new clients get the latest state of the node, instead of the latest message. Like Flexdash used to do it.

This could be a far more effective way of storing the data vs. how we do it today. My concern here is that it then requires every node to define a schema of what it's "state" looks like. Currently, this is not required, we store the raw msg objects, and so can just utilise the same logic as in onInput to parse the msg, but in onLoad.

@omrid01
Copy link
Author

omrid01 commented Dec 19, 2023

Differentiate onLoad as a function, from onInput

Excellent. Will we be able to assume that the onLoad function is called after all HTML objects finished loading? (Currently, when I place my on-load code in the beginning of a Template's JavaScript section, I need to do an async wait loop for this).

Datastore has both save (default) and append options

  • The default just saves the last msg received against the node. The append option however stores the history of msg, such that the history can be used onLoad to re-populate the content, e.g. ui-chart

This could work for objects like ui-chart (saving X last messages), but other use-cases can be problematic (e.g. a grid with messages which add & remove data, or action/trigger messages). My suggestion is to determine the datastore behavior per msg, via a designated node-configuration property (overridable by a dynamic message property). It can either be a literal (None, Replace, Append, Clear etc.) or a number representing what to do with the message (0=do not save, 1=replace with this msg, X=append to last messages (truncate older if more than X), -1 = clear all saved messages).
In any case, the actual msg replay MUST be optional. Either via node-configuration checkbox, or an API which can be called at-will from inside the onLoad function. (in dashboard V1, @bartbutenaers and me had to add some ugly code to detect & discard these unasked-for auto-replayed messages).

I think I like this. Will be a mission to implement (needs to be duplicated across all core UI nodes), and would also then require all third-party widgets to implement this too though, which makes me hesitate.

It will be probably good enough to add this just to ui-templates, as this is what multi-user implementations are using.

@joepavitt
Copy link
Collaborator

Will we be able to assume that the onLoad function is called after all HTML objects finished loading?

You are correct

My suggestion is to determine the datastore behavior per msg, via a designated node-configuration property

This is getting very complicated, very quickly, with a lot of detailed configuration options on all nodes. I am hesitant.

It will be probably good enough to add this just to ui-templates, as this is what multi-user implementations are using.

Just so that I understand your own motives here, are you coming from the perspective of a third-party developer for widgets, or user/developer utilising ui-template?

@omrid01
Copy link
Author

omrid01 commented Dec 19, 2023

This is getting very complicated, very quickly, with a lot of detailed configuration options on all nodes. I am hesitant.

I agree. A far as I'm concerned, I'm better off with no message storing/replay at all for template nodes. It's more hassle than help and giving us a big headache in Dashboard V1.

Just so that I understand your own motives here, are you coming from the perspective of a third-party developer for widgets, or user/developer utilising ui-template?

I'm chief solution architect, implementing Node-red (and loving it), in more than 20 of our company's projects. We are using ui-template for 2 main purposes:

  1. Advanced multi-user UI forms
  2. Custom UI for monitoring & runtime control, on top of Node-red

We also developed many custom nodes (for internal use), but not for UI

@joepavitt
Copy link
Collaborator

A far as I'm concerned, I'm better off with no message storing/replay at all for template nodes. It's more hassle than help and giving us a big headache in Dashboard V1.

This I can get on board with. When chatting with @MarianRaphael about this topic, we settled on core Dashbaord 2.0 having two states:

  1. Default - as is now, each node has a known "state" defined by it's own behavioural logic/code
  2. Stateless - whereby we do not store anything server-side, and state management is user/database-dependent

Expect this is (easily) configured at the UI/Page level, more complicated on a node-by-node basis.

@omrid01
Copy link
Author

omrid01 commented Dec 19, 2023

This I can get on board with. When chatting with @MarianRaphael about this topic, we settled on core Dashbaord 2.0 having two states:

  1. Default - as is now, each node has a known "state" defined by it's own behavioural logic/code
  2. Stateless - whereby we do not store anything server-side, and state management is user/database-dependent

Expect this is (easily) configured at the UI/Page level, more complicated on a node-by-node basis.

Fine with me. Is it possible to separate this page-level config into 2: one for ui-templates, and one for all other UI nodes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request that needs to be turned into Epic/Story details needs-triage Needs looking at to decide what to do
Projects
Status: Backlog
Development

No branches or pull requests

3 participants