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

Prefix Scans for Storage Keys #31

Open
dwmunster opened this issue Oct 8, 2019 · 3 comments
Open

Prefix Scans for Storage Keys #31

dwmunster opened this issue Oct 8, 2019 · 3 comments

Comments

@dwmunster
Copy link
Contributor

dwmunster commented Oct 8, 2019

Summary

Since many uses of the Storage interface may have hierarchical keys, e.g. the permissions, it would be beneficial to have a way to iterate over keys matching a given prefix.

Basic example

for _, key := range a.store.KeysWithPrefix("joe.permissions.mymodule.") {
  ...
}

Motivation

From #28

Since we are iterating over all keys the bot knows we should expect that also non-permission keys are among them.

I suggest you update your unit test to check this is also handled correctly.

Rather than push this onto any developer who needs to interact with the permissions, we could require the Storage interface to support prefix scans. Some storage engines may have a higher performance way of doing prefix scans and we could take advantage of that. If the storage engine does not offer it, the bot could handle the naive looping over keys itself. This removes the burden from developers to implement/test this independently.

Potential Issues

  • If the prefix scan isn't optional for the Storage interface, a new release would break any third party implementations. As described above, this would not be an issue.
@dwmunster
Copy link
Contributor Author

If you're amenable, I can throw together a PR for this.

@fgrosse
Copy link
Contributor

fgrosse commented Oct 20, 2019

Yes, I think this addition is a good new feature. You still want to submit a PR for this @dwmunster ? I have more time to work on features and review PRs in the next two weeks. Let me know if you don't have the time right now so I will give it a shot :)

@dwmunster
Copy link
Contributor Author

Sorry for taking a while to respond @fgrosse, I've gotten quite busy these past couple weeks. I should have time to put something together this weekend if you'd like. If you'd like it done before then, have at.

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 a pull request may close this issue.

2 participants