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

Storage documentation is incorrect #682

Open
leighman opened this issue Feb 28, 2017 · 7 comments
Open

Storage documentation is incorrect #682

leighman opened this issue Feb 28, 2017 · 7 comments

Comments

@leighman
Copy link

@leighman leighman commented Feb 28, 2017

In the storage section of the README it mentions that storage backends should implement delete methods.
None of the available backends implement delete nor can I find it in the internal implementation.
The documentation needs to be updated presumably.

@barlock
Copy link

@barlock barlock commented Mar 7, 2017

There is at least an implementation in "simple storage" but I am confused as to what the intended api should be. The implementation I linked to says that delete should take an id but in actuality it takes the whole object. The test cases don't actually use delete so I can't tell what they think it should be. Any insights?

@barlock
Copy link

@barlock barlock commented Mar 7, 2017

I can confirm that all of the botkit-storage-* in the howdyai org don't seem to implement delete. Redis implements remove though and takes an id rather than the whole object

@peterswimm
Copy link
Contributor

@peterswimm peterswimm commented Sep 21, 2017

Is this fixed? #854

@pedrorocha-net
Copy link

@pedrorocha-net pedrorocha-net commented Nov 5, 2017

I'm not sure from the comments: should we have delete methods? I'm needing them on MongoDB storage and i'm not sure if I should create a PR here in the project or should place it only inside my custom code.

What do you think?

@fastbean-au
Copy link
Contributor

@fastbean-au fastbean-au commented Oct 2, 2018

The Jabber bot looks like it requires both 'delete' and 'all' to be implemented for storage methods. CoreBot already checks for the existence of 'get' and 'save', but not 'delete' and 'all'. Checks for 'delete' and 'all' could be added to the CoreBot (which might be a breaking change), placed in the Jabber bot, or the documentation updated to include either indicating that Jabber will only work with some storage mechanism implementations, or for the storage mechanisms indicating what methods that they should implement.

It looks like the Mongo storage implements all four methods now, as does Redis (except that 'delete' is 'remove'), and Datastore and Firebase do not implement 'delete'.

@fastbean-au
Copy link
Contributor

@fastbean-au fastbean-au commented Oct 3, 2018

Looking at the other plugins (those listed here):

CouchDB and Datastore support all four methods
Firestore, MySQL and Postgres (not maintained) do not support 'delete'

@benbrown benbrown self-assigned this Oct 19, 2018
@benbrown
Copy link
Contributor

@benbrown benbrown commented Oct 19, 2018

hey @fastbean-au are you a member of community.botkit.ai? you should join if not!

@benbrown benbrown added the legacy label May 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.