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

Save State #10

Open
balajijegan opened this issue Sep 7, 2018 · 3 comments
Open

Save State #10

balajijegan opened this issue Sep 7, 2018 · 3 comments

Comments

@balajijegan
Copy link
Contributor

  • Currently the save state is called only on stop. If the bot terminates abnormally for whatever reason, this can cause state to be lost. Case in point when the bot has been running for several days.

https://github.com/arcticfoxnv/slackminion/blob/e22601353f82ea2395a32933f4c2355374957d0b/slackminion/bot.py#L137

  • Propose setting a timer to save state periodically. May be once a day.

Thoughts?

@balajijegan
Copy link
Contributor Author

I dont know how I would do this either, so appreciate feedback or pointers on how to do this.

@arcticfoxnv
Copy link
Collaborator

A periodic save would definitely be good to have. It was probably just an oversight on my part that it doesn't currently do so. Current thinking is that plugins can start timers which run in separate threads, so we could probably use that to save state.
https://github.com/arcticfoxnv/slackminion/blob/e22601353f82ea2395a32933f4c2355374957d0b/slackminion/plugin/base.py#L65-L72
We could either update the core plugin, or add a new plugin (maybe in slackminion/slackminion/plugins/state/) to schedule timers for saving state. I suppose it doesn't have to be in a plugin, but I want to try and keep as much in plugins as possible, so bot functionality can be more easily customized as needed.

@amckenna-pinterest
Copy link
Contributor

The state should be saved anytime a savable plugin attribute is updated. Otherwise there is a risk of state rollback if the bot terminates unexpectedly. I've added this as a one-off to some of our internal plugins but it should really be implemented in BasePlugin I think. I'll try to work on this sometime soon.

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

No branches or pull requests

3 participants