launchdarkly / node-server-sdk

LaunchDarkly Server-side SDK for Node
Other
79 stars 65 forks source link

Support Redis Cluster store via ioredis #122

Closed seanxiesx closed 1 year ago

seanxiesx commented 6 years ago

https://github.com/luin/ioredis

rmanalan commented 5 years ago

@eli-darkly I'm going to take a stab at swapping out redis with ioredis. We need this as well.

eli-darkly commented 5 years ago

@rmanalan We hadn't had a chance to look into this yet, so I'd certainly be curious whether you run into any problems. Thanks.

eli-darkly commented 5 years ago

@rmanalan By the way, as of v5.6.1 there is an optional new helper mechanism for building a feature store integration that makes things quite a bit simpler. You can take a look at https://github.com/launchdarkly/node-dynamodb-store for an example of how it can be used. In that example, it specifies the Node SDK as a peer dependency so it does actually require that v>=5.6.1 be present, but if you want to be more backward-compatible you could also just pull in the Node SDK code as a regular dependency so as to use the helper stuff from it.

rmanalan commented 5 years ago

@eli-darkly I have a working version using ioredis, however, I need to make it generic so that it doesn't break existing users. Also, I introduced a lock mechanism so that in clustered multi-process environments, you only have one process that writes to redis at a time. This might eliminate the need to run ld-relay in large clustered environments. I'll work on a PR soon.

eli-darkly commented 5 years ago

@rmanalan I'm not sure what you mean by "I need to make it generic"; could you clarify?

rmanalan commented 5 years ago

@eli-darkly our service uses Electrolyte for DI. Redis in our env is defined as a component. So, my version of the redis feature store doesn't actually connect to redis... it just uses the already instantiated component. So, what I mean by making it generic is I need to make sure that you can either pass in an already instantiated redis client or the standard connection params that the original redisFeatureStore supports. I think this is a good addition anyway since in most environments devs might want to use an existing client.

eli-darkly commented 5 years ago

@rmanalan Got it. Yes, we've generally taken that approach in our other SDKs, but I guess we didn't think of it in the Node one.

kinyoklion commented 1 year ago

@seanxiesx

The new version of the SDK, v8, supports ioredis.

The new version has a new location and package.

The new version is located here: https://github.com/launchdarkly/js-core/tree/main/packages/sdk/server-node It also has a new package name, @launchdarkly/node-server-sdk.

The redis package is also moved and renamed.

Github: https://github.com/launchdarkly/js-core/tree/main/packages/store/node-server-sdk-redis Npm: https://www.npmjs.com/package/@launchdarkly/node-server-sdk-redis

Thank you, Ryan

github-actions[bot] commented 1 year ago

This issue is marked as stale because it has been open for 30 days without activity. Remove the stale label or comment, or this will be closed in 7 days.