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

Change storage strategy #8

Closed
wants to merge 7 commits into from

Conversation

oscarnevarezleal
Copy link

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

@roccomuso
Copy link
Owner

roccomuso commented Feb 7, 2017

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 Extend the persistence layer to support more DB #4

@oscarnevarezleal
Copy link
Author

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.

@roccomuso
Copy link
Owner

@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).

@oscarnevarezleal
Copy link
Author

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

@MeStrak
Copy link

MeStrak commented Apr 5, 2020

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?

@roccomuso
Copy link
Owner

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

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