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

Add support for third party session handlers #137

Closed
wants to merge 2 commits into from

Conversation

kirsle
Copy link
Member

@kirsle kirsle commented Jul 3, 2016

This adds a constructor option sessionHandler which provides a hook for third party developers to implement their own session stores (e.g. using Redis or something).

The default session handler, from src/sessions.coffee implements the in-memory store that RiveScript previously used. All internal code references to user variables from RiveScript._users have been modified to go through the public setUservar() function and friends. These functions then delegate to the equivalent functions from the session handler. The core RiveScript library provides no other session handlers, but some may be added as examples in the eg/ directory in the future.

Session handlers aren't required to implement all functions (the unit test tries setting a "null" session handler which gives the bot really bad amnesia).

Fixes #135

@gleuch
Copy link
Contributor

gleuch commented Aug 4, 2016

👍 Great!

@julien-c
Copy link
Contributor

👍 Nice. Do you know whether you will merge and release this soon?

@kirsle
Copy link
Member Author

kirsle commented Aug 22, 2016

I'm not sure how useful this was after all. It won't work with any async session drivers, which sort of defeats the original purpose for this code. Somebody wanted to hook up a database layer (Mongo or Redis or SQL or something) to manage the sessions, but most of those things work asynchronously using promises or callbacks, but that can't work with the way rivescript-js is currently designed.

@julien-c
Copy link
Contributor

Got it. I'm in the process of implementing the way you're doing it in the persistence example, looks like it's going to be fine. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants