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

Add PouchDB Context store plugin #1

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

KazuhiroItoh
Copy link

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

This makes it possible to store context data to PouchDB.

The design note is https://github.com/node-red/designs/blob/master/designs/pouchdb-context-plugin.md

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality


- Data is saved in the JSON object format supported by PouchDB. The plugin does not convert JSON data to a string for storage.

Code example that references database data :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So where does this code sit ? Is this totally external to Node-RED ? Or is it (or should it be) an example in a function node ? or ???

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code example is supposed to be executed externally, and can also be run in the function node.
When executing in the function node, set the functionGlobalContext in setting.js so that PouchDB can be used in the function node.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe worth adding that as a "how-to" in the readme. (probably more applicable to the events example though).

This allows you to back up the data stored in your local SQLite to a remote CouchDB.This allows you to back up context data stored in your local SQLite to a remote CouchDB.

- In an environment with multiple context stores, only contexts using the PouchDB plugin will be backed up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise where does this code sit ? I see that it creates events ? Can they be used in Node-RED ?
If I had two machines replicating some part of global context ... how could I get a trigger on machine B that said that machine A had updated my context, so I should re-read it, and update my dashboard, for example ?

@dceejay
Copy link
Member

dceejay commented Jun 22, 2021

Not sure if this is in this plugin or in core... If I use a function node to try to get from the context it fails with a "callback must be a function" error. It works fine with change node. Works OK if I specify "memory" or "file" as second parameter.

Simple test case

[{"id":"fca4c86710ca190f","type":"inject","z":"68be502bf586a0e3","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payloadType":"date","x":240,"y":940,"wires":[["37b34242b8f8fccc"]]},{"id":"37b34242b8f8fccc","type":"change","z":"68be502bf586a0e3","name":"","rules":[{"t":"set","p":"#:(pouchdb)::foo","pt":"flow","to":"payload","tot":"msg"}],"action":"","property":"","from":"","to":"","reg":false,"x":470,"y":940,"wires":[[]]},{"id":"e6c96d48be8b9897","type":"debug","z":"68be502bf586a0e3","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","statusVal":"","statusType":"auto","x":730,"y":1000,"wires":[]},{"id":"f402c98089596568","type":"inject","z":"68be502bf586a0e3","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payloadType":"str","x":230,"y":1000,"wires":[["fb5d4ffcde32918b"]]},{"id":"2f58e60558bfe1ff","type":"debug","z":"68be502bf586a0e3","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","statusVal":"","statusType":"auto","x":730,"y":1060,"wires":[]},{"id":"28eadbe603dcbd8e","type":"inject","z":"68be502bf586a0e3","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payloadType":"str","x":230,"y":1060,"wires":[["33702a0f0c6b76d1"]]},{"id":"33702a0f0c6b76d1","type":"function","z":"68be502bf586a0e3","name":"","func":"msg.payload = flow.get(\"foo\",\"pouchdb\");\nreturn msg;","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"x":460,"y":1060,"wires":[["2f58e60558bfe1ff"]]},{"id":"fb5d4ffcde32918b","type":"change","z":"68be502bf586a0e3","name":"","rules":[{"t":"set","p":"payload","pt":"msg","to":"#:(pouchdb)::foo","tot":"flow"}],"action":"","property":"","from":"","to":"","reg":false,"x":480,"y":1000,"wires":[["e6c96d48be8b9897"]]}]

I'm guess something is tripping up around
image

@KazuhiroItoh
Copy link
Author

KazuhiroItoh commented Jun 23, 2021

PouchDB plugin and Redis plugin only supports asynchronous access.
File plugin has synchronous access when the cache is enabled, but only asynchronous access when it is disabled.

This flow supports asynchronous access.

[{"id":"fca4c86710ca190f","type":"inject","z":"8254084d.010598","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payloadType":"date","x":160,"y":60,"wires":[["37b34242b8f8fccc"]]},{"id":"37b34242b8f8fccc","type":"change","z":"8254084d.010598","name":"","rules":[{"t":"set","p":"#:(pouchdb)::foo","pt":"flow","to":"payload","tot":"msg"}],"action":"","property":"","from":"","to":"","reg":false,"x":390,"y":60,"wires":[[]]},{"id":"e6c96d48be8b9897","type":"debug","z":"8254084d.010598","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","statusVal":"","statusType":"auto","x":650,"y":120,"wires":[]},{"id":"f402c98089596568","type":"inject","z":"8254084d.010598","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payloadType":"str","x":150,"y":120,"wires":[["fb5d4ffcde32918b"]]},{"id":"2f58e60558bfe1ff","type":"debug","z":"8254084d.010598","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","statusVal":"","statusType":"auto","x":650,"y":180,"wires":[]},{"id":"28eadbe603dcbd8e","type":"inject","z":"8254084d.010598","name":"","props":[{"p":"payload"}],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","payloadType":"str","x":150,"y":180,"wires":[["33702a0f0c6b76d1"]]},{"id":"33702a0f0c6b76d1","type":"function","z":"8254084d.010598","name":"","func":"flow.get(\"foo\",\"pouchdb\", function(err,value){\n    if(err){\n        node.error(err, msg);\n    } else {\n        msg.payload = value;\n        node.send(msg);\n        node.done();\n    }\n});\nreturn;","outputs":1,"noerr":0,"initialize":"","finalize":"","libs":[],"x":380,"y":180,"wires":[["2f58e60558bfe1ff"]]},{"id":"fb5d4ffcde32918b","type":"change","z":"8254084d.010598","name":"","rules":[{"t":"set","p":"payload","pt":"msg","to":"#:(pouchdb)::foo","tot":"flow"}],"action":"","property":"","from":"","to":"","reg":false,"x":400,"y":120,"wires":[["e6c96d48be8b9897"]]}]

@dceejay
Copy link
Member

dceejay commented Jun 23, 2021

OK- could the error message be changed to suggest that possibility ? Not that is must be a function - but that it must exist :-) - Maybe an extra test - if callback === undefined ... say that it is async and a callback must be defined

    if (callback === undefined){
        throw new Error("Async storage - a callback must be defined");
    }

Or something ???

@knolleary
Copy link
Member

Isn't the error coming from the core?

@dceejay
Copy link
Member

dceejay commented Jun 23, 2021

no - it's coming from that line 160 highlighted above (so no i18n :-) )
certainly if I insert that snippet just before it gets called fine if I omit the callback.

@KazuhiroItoh
Copy link
Author

throw new Error("Async storage - a callback must be defined");

Change the message to the suggested one.
Do the same for the Redis plugin.

@dceejay
Copy link
Member

dceejay commented Jun 24, 2021

I think you need to add it as well... so if undefined throw that error - and if defined but not a function then throw the error that you were doing.

@KazuhiroItoh
Copy link
Author

Two updates have been made to address your suggestions.

  1. Improved error message
    Improved the error message when callback is not specified in get() to an appropriate message.
    If the callback is set to a value other than the function, the error message before the change will be printed.
    This is checked in Runtime/lib/nodes/context/index.js before the plugin is called.

  2. Add replication execution example in function node
    Added a replication execution example in the function node to the README.
    This is an example of using a remote CouchDB to replicate the context of machine A to machine B.
    The following conditions are applied.
    The replication flow is executed manually.
    Do not update the context at the time of execution.

@dceejay
Copy link
Member

dceejay commented Jul 15, 2021

👍 (I notice you also applied same "fix" to redis plugin... thanks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants