`redis` adaptor

[josephjclark]

A design for a minimal redis adaptor

Overview

get(key)
hget(key, field)
hgetAll(key, field)
set(key, value) // value can be an object and _replaces_ the existing entry at the key
hset(key, field, value) 
scan(pattern, options) // keys returned to state.data

command(commandName, args) // generic call to client.sendCommand, allowing arbitrary command execution. Results are written to state.data.

Auth

Basic and oath?

Implementation notes

We should use the node-redis client

~See @redis/search for the search API - I suggest we just wrap this. search should return the documents array to state.data (don't return the meta wrapper)~

[josephjclark]

I suspect we should use pipelining or transactions to optimise large setters. Ideally this should be under-the-good but we may need users to tell us when to start and finish.

I'm researching those topics at the moment - but they are an optimisation.

Apparently the node redis client does autopiplining and will pipeline all requests in the same tick. See the docs. We should think about leveraging this (although next tick likely won't work because we have a very asynchronously runtime)

[josephjclark]

We should be careful not to over-optimise. Maybe we should just put out simple API above and see how well that performs. If upserts are slow we can can work out how to optimize

[josephjclark]

Because redis's API is quite big, we could maybe have a exec() operation which sends a raw redis command up to the server. The node client has a sendCommand API to do just this.

[mtuchi]

For Auth: It will be similar to other database.

redis[s]://[[username][:password]@][host][:port][/db-number] (see redis and rediss IANA registration for more details)

createClient({
  url: 'redis://alice:foobared@awesome.redis.server:6380'
});

Redis Configuration Schema

``` { "$schema": "http://json-schema.org/draft-07/schema#", "title": "Redis Database Connection Schema", "type": "object", "properties": { "host": { "type": "string", "description": "The hostname or IP address of the Redis server.", "pattern": "^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$|^([a-zA-Z0-9-]+\\.)+[a-zA-Z]{2,}$" }, "port": { "type": "integer", "description": "The port number on which the Redis server is listening.", "minimum": 1, "maximum": 65535, "default": 6379 }, "username": { "type": "string", "description": "The username for authenticating with the Redis server.", "minLength": 1 }, "password": { "type": "string", "description": "The password for authenticating with the Redis server.", "minLength": 1 }, "dbNumber": { "type": "integer", "description": "The database number to connect to on the Redis server.", "minimum": 0 } }, "required": ["host"], "additionalProperties": false } ```

[mtuchi]

For testing locally use, setup a redis server using docker 👇🏽

docker run -p 6379:6379 -it redis/redis-stack-server:latest

[mtuchi]

set(key,value), Hiya @josephjclark looking at the redis-readme. It looks like set should take 3 params

client.hSet('key', 'field', 'value');
//Using js object
await client.set('key', 'value', {
  EX: 10,
  NX: true
});

[mtuchi]

@josephjclark just a heads up, i don't think we can unit test these new functions 😢. I don't know how i can setup a mock client for redis

[josephjclark]

I don't know about that third argument @mtuchi - that's just a list of options to drive the set. Stuff like timeout and upsert control. Maybe we can expose an options object later but my feeling is that it's a level of detail we don't need to cover. It's too low.

Which reminds me: the value in the set should support a JSON object (in which case the existing object is replaced).

[josephjclark]

I can help you with a mock pattern. We need something a bit like common http where we set the mock client externally

[mtuchi]

@josephjclark yeah that will be nice, i will be working on this tomorrow

[mtuchi]

EOD Updates: Loved my experience with redis so-far, the documentation is easy to understand and easy to test.

Progress

• Manage to connect to the Redis Cloud Server
• Download and use Redis Insight which is very useful for visualizing redis database
• Generate a redis adaptor and start adding get() and set()
• Locally run a job that uses get() and set()

Findings

• As i was planning for search() implementation i stumbled upon this requirement. Before we can perform any searches, we need to create RedisSearch index using client.ft.create(). You can find the instruction here. I need @josephjclark input on this it seems very important if people need to use search()

• Current implementation of set() uses client.set(). In the docs SET support options as well.

  • Qn for @josephjclark Do we want to setup options ? set(string: key, string: value, object?: option)
  • Also sets()only sets a key to hold the string value it does not support hash values. To add has values you need to use client.hSet()

• Current implementation of get() Returns only the string value of a key since we i am using client.get() function behind the scene. To return hash values we need to use one or both of the following approaches

  • Use client.hGet(key, value) -> Returns the value associated with field in the hash stored at key.
  • Use client.hGetAll(key) -> Returns all fields and values of the hash stored at key. In the returned value, every field name is followed by its value, so the length of the reply is twice the size of the hash.
  • Use client.hVals(key) -> Returns all values in the hash stored at key

    I need @josephjclark help on carefully planning for thinking

~Next Steps~ Some-Thoughts

Before i make a mess i think it's best for me and @josephjclark to sync on the design for this adaptor because we need to address my findings ☝🏽.

Based on my quick understanding of the NH 1st workflow

• I think i need set(string: key, object: value, object?: options) to be able to import incoming message form RTM devices
• I haven't thought much about 2nd workflow but it looks like search() is what we need.
• According to Benson email it looks like they were thinking of using scan commands instead maybe @aleksa-krolls if you can reach out to Benson and ask him if their use-case will work well with search()?

[josephjclark]

My thoughts @mtuchi (happy to take a call)

I want to do the minimal and simplest amount of work here. So:

• It sounds like Benson needs scan, not search. Scan tests for patterns on keys. Search needs an index to be built (which I don't think is what Benson wants)
• So forget search for now. Let's add support for scan
• For the record, I think indexes are a db admin issue. Someone needs to create the index before the workflow runs. Maintaining indexes as part of the workflow feels entirely wrong to me.
• Let's support set, hset, get and hget for now.
• I am interested in exploring sometihng like get(key, field?) and set(key, obj, { shallow: boolean }), where a shallow set is like a hset (sets keys) and a non-shallow set is just a set (replaces the while object). I think this is good for humans but maybe bad for redis experts (of which we have none, to be fair). Let's worry about this later.
• We should not support options on set until we absolutely have to.

Let me update the spec.

[mtuchi]

Thank you @josephjclark i will pick this up tomorrow

[mtuchi]

@josephjclark why callback in scan(pattern,callback, options) is the 2nd argument ?

[josephjclark]

New design

because redis doesn't work like I thought, I want to make this an even lower level API

We can't do useful callbacks or auto fetching in scan. And on reflection the callback won't work either.

So we should return the keys to state.data, then you can do each() on them and get or hget or hgetall or whatever you want todo

[mtuchi]

EOD Update: See work in progress #706

I made good progress on this issue, as per our discussion with @josephjclark hGetAll() is the last function to be implemented for this issue. The pending item for this issue are 👇🏽

• [x] Properly document each function and including option's
• [x] Unit test, I know it's a hard one perhaps @josephjclark you can help me with the approach for creating a fake client so alt-east we have couple of unit test.
• [x] Update changelog
• [x] Update redis logo
• [ ] Create a PR for adding redis on marketing website

[mtuchi]

This is waiting for another round of feedback from @josephjclark See work in progress #706

[mtuchi]

@aleksa-krolls redis v1.0.0 is out, I have created a PR on marketing website https://github.com/OpenFn/marketing/pull/190

[mtuchi]

Based on @aleksa-krolls suggested next steps:

Mtuchi to investigate why we can’t use the JSON data type to minimize the manipulation needed to store the data received via webhook. (FYI see ChatGPT response on how you can use SCAN to search JSON)

Currently in nh-workfow, i am using the hash data type when importing incoming payload to DB, and query the data using hGetAll(). The challenge with hash data type requires the json object to be a string of key value pair. If the object have other data type nested inside like array or json you have to convert these values into string. So i am using JSON.stringifigy to convert an array of records into a string. @aleksa-krolls suggest if we can set the JSON data type directly instead of using HASH data type, which is possible when using client.json.set(key, $, {a,b,c}) and you can read the data using client.json.get(key). @josephjclark I would love you input on the best way to design those helpers

Something to note is that JSON data type is bigger than HASH data type. about 3x bigger with my current test data.

Mtuchi to troubleshoot why he can’t scan the TS ':20240524T012736Z (from key f9eb9cc8c26d4697bd741928719ac87c:20240524T012736Z:20240524T031736Z)

There are couple of improvements on scan() operation

• We should remove type default value
• We should use client.scanIterator() instead of client.scan() to avoid implementing autoFetch if the return cursor value is not 0. @josephjclark will share more details on this when we sync

[josephjclark]

Let's add json support like this:

jGet(key)
jSet(key,  json)

jGet and jSet (like hSet and hGet) allow json objects to be read and written.

For now, we always read/write from the root path. Maybe later we'll add a path option.

This always replaces the object at that key. This needs to be clearly documented.

[josephjclark]

Changes to scan():

• scan needs to iterate the whole database (and we need to document this!)
• scan should not take cursor parameters (it's too hard to loop in job code, so let's just do it in the adaptor function).
• We can take count as an argument in case users want bigger "pages". This allows for some optimization.
• Type should not have a default value
• We should support the json type and map it to the redis internal type

I'd love a good unit test on the updated scan but that depends a bit on how easy it is to mock out the scan API. Maybe we can mock the iterator to return three "pages" of results?

Again, we'll likely evolve and improve this API in future

[mtuchi]

Thanks @josephjclark for the help, I have created two issues for redis

• https://github.com/OpenFn/adaptors/issues/710
• https://github.com/OpenFn/adaptors/issues/709

As for next step i will add estimate on those issue and sync with @aleksa-krolls on which sprint should we put them