node-webhooks icon indicating copy to clipboard operation
node-webhooks copied to clipboard

Change storage strategy

Open oscarnevarezleal opened this issue 8 years ago • 6 comments

Change storage strategy with a few modifications to base source. FileStorage & Redis storage are now available, new strategies are easy to integrate.

oscarnevarezleal avatar Feb 07 '17 07:02 oscarnevarezleal

Just a few things, usually when there are multiple core changes, they should be previously discussed.

  1. backward compatibility (should start from 4.4.0 at least)
  2. Why did you introduce a ready() method?
  3. We should discuss an abstraction layer for the storage.
  4. good the idea of a multi() method
  5. Actually I don't like this API: const fileStorage = require('./src/storage').get('file', fileConfig) It's too verbose. Pheraps I'd suggest something like #4

roccomuso avatar Feb 07 '17 16:02 roccomuso

Hi and thanks for the quick reply. I haven´t see your comments until now.

I know core changes must´ve been previously discussed.

A little background, To be honest I never intended to share the fork in first place, the changes fullfill its purpose on my project and then I tought It be a nice thing to share it with you.

2 - Storage channel isn´t always ready when attempt to use it ( Redis for instance ) 3 - Absolutely, the Json and DB references were all across the project. It was very difficult to abstract. 5 - Yeah me neither, come to it while programming usnig bottom to top approach.

oscarnevarezleal avatar Mar 13 '17 19:03 oscarnevarezleal

@oscarnevarezleal we could start discussing for an abstraction layer looking for hints from other npm modules that uses multiple storage for their purposes (like express-session).

roccomuso avatar Mar 13 '17 22:03 roccomuso

@roccomuso sure thing, how do you like to start?

oscarnevarezleal avatar Mar 14 '17 03:03 oscarnevarezleal

Hi @roccomuso, @oscarnevarezleal I might start using this library and it would be great to be able to use redis. More generally, is the project still active?

MeStrak avatar Apr 05 '20 22:04 MeStrak

@MeStrak it is but still don't officially support Redis

roccomuso avatar Apr 05 '20 22:04 roccomuso