Support for a data store that is implemented via promises

[mwildehahn]

I'm implementing a redis data store and have all of the methods on the data store resolving via promises.

I'd like to adjust all of the methods that interact with the datastore to support potentially resolving a promise if one is present.

If I get a +1 I''ll submit a PR.

[DEGoodmanWilson]

I'd be interested in seeing how this is implemented. I'd like to extend the cacheing system to be a little more flexible in the backing layer used (as well as turning it off when not wanted), so it would be fun and informative to see what you have!

[ghost]

Yep, this sounds great. The wrinkle here is that you'll also need to adjust the way the store is called from the RTM API, otherwise you can pass an event that the developer would expect to have cached data that's fetchable from the data store, but which is handled before the data store promise is resolved.

[mwildehahn]

cool, i'm going to implement something similar to https://github.com/graphql/graphql-js/blob/39744381d5173795d3b245dcb5d86e78bb3638fe/src/execution/execute.js#L920, ie. isThenable, but need to play with some ways not to make this a pain in all of the message handlers

[mwildehahn]

hm, this is going to be more annoying than i thought within the message handlers, especially ones like this: https://github.com/slackhq/node-slack-sdk/blob/master/lib/data-store/message-handlers/user.js#L14 where you could technically have a mix of promise/non promise methods on the store.

the cleanest thing to do IMO would be to change the signature of SlackDataStore methods to always return a promise. then implementing a store backed by a db is much simpler. but that would need to go in a 4.0 release and i'm not sure how you all feel about that.

I implemented a redis data store here: https://github.com/lunohq/slack-redis-data-store. I mostly just want to cache the rtm start data so am going to use an adjusted MemoryDataStore that also writes to this redis store, but only for the https://github.com/slackhq/node-slack-sdk/blob/master/lib/data-store/data-store.js#L387 method.

In my ideal world I would setup something like:
MultiWriteDataStore, a data store that gets init'd with an array of stores. All reads will come from the first store provided, all writes will be sent to all the stores in the array. This would let the local process read from a memory store and support propagating changes to a redis store which can be used by other processes within my app. ie:

const dataStore = new MultiWriteDataStore([new MemoryDataStore(), new RedisDataStore()])

[DEGoodmanWilson]

@mhahn A hypothetical question: Would it be easier to implement something like this at a different layer? Perhaps if we provided a middleware architecture for cacheing?

[mwildehahn]

I think the data store is the right layer to do this. It already does a great job of reacting to changes sent from the RTM client and keeping the state in sync, the only issue is it isn't built to support remote stores. I don't think it would be hard to fix, but doing so in a way that doesn't change the signature of the data store interface seems like more work than it's worth.

[DEGoodmanWilson]

There are enough other issues with the existing interface anyway that a re-write of this layer might well be called for, so I'm not really bothered by this :)

[mwildehahn]

Cool, I'm happy to help with this. Are there other issues you can point me to? @l12s do you have any words of wisdom before starting out on this?

[DEGoodmanWilson]

#111 is the biggie here.

[DEGoodmanWilson]

@acemtp you might enjoy this conversation, BTW.

[aoberoi]

closing as duplicate of #410. @mwildehahn, if you're still interested, i'd love your feedback and continued help in that issue.