Mattermost-server: Add KVListWithOptions to p.Helpers

Created on 26 Oct 2019  路  18Comments  路  Source: mattermost/mattermost-server

Often, plugins need to find all keys of a type.

Suggest implementing

func (h *Helpers) KVListAll(options ...Options) ([]string, error) ([]string, error)

For now there should be two options:

  • Prefix(prefix string)
  • WithChecker(f func (key string) (continue bool, err error))

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-19732

AreToolkit Easy Help Wanted PR Exists TecGo

Most helpful comment

Currently making an attempt to use code from this PR here. So far so good :)

All 18 comments

I want to help with this.

Thanks @a8uhnf :+1:

@levb What do you think about changing the signature to KVListPrefix(prefix string, page, perPage int) to be consistent with our other API methods?

@hanzei the whole point of this helper is to enumerate the entire set of keys of a type, without having to bother with the paging. 1/5 paging should be internal to the implementation. An overall limit of items to return should be enough. Maybe KVListAllWithPrefix?

I also contemplated a KVPrefixDo that would take a function, but decided against it for simplicity.

Often, plugins need to find all keys of a type.

@levb Can you please point me to an example plugin where this is required?

@RajatVaryani Here's a pending PR, this request came out of it. It has 2 examples, grep for KVList, https://github.com/mattermost/mattermost-plugin-jira/pull/351/files#diff-3b02caad0024138fae666f1168b4d0cdR487.

This (first) example performs extra filtering, thus my comment about a KVPrefixDo being (maybe) a better way to express the functionality. 0/5 maybe implement them both?

@hanzei the whole point of this helper is to enumerate the entire set of keys of a type, without having to bother with the paging. 1/5 paging should be internal to the implementation. An overall limit of items to return should be enough. Maybe KVListAllWithPrefix?

I agree and like KVListAllWithPrefix. What is the use case for a limit?

@hanzei Made me think, I agree, let's simplify to func (h *Helpers) KVListAllWithPrefix(prefix string) (keys []string, err error).

Opinion on a supplemental/underlying func (h *Helpers) KVDoWithPrefix(prefix string, func (key string) (continue bool, err error)) error?

:+1: func (h *Helpers) KVListAllWithPrefix(prefix string) (keys []string, err error)

The name KVDoWithPrefix confuses me. What does the Do mean? Is it a KVListAllWithChecker?

Maybe we want a KVListAll(options ...Options) ([]string, error) and support various options like Prefix(prefix string) or WithChecker(f func (key string) (continue bool, err error)). What do you think?

馃憤 to options in this use-case! KVListWithOptions FTW.

@a8uhnf Do you have any questions? Is the ticket clear to you?

Hi @a8uhnf, let us know if you're still open to working on this one? Let us know if you have any questions!

In the meantime, I'll open this up for grabs :)

I'd like to help out on this one.

Awesome, thank you @dlclark! Let us know if you have any questions

For this ticket is there an expectation of refactoring the commit mentioned by @levb (https://github.com/mattermost/mattermost-plugin-jira/pull/351/files#diff-3b02caad0024138fae666f1168b4d0cdR487) to use the new functionality?

Left to my own devices I'd think it'd be a separate ticket, but since I'm new here I don't want to assume too much.

It would be great, if you would refactor the code in the jira plugin. But you'r PR alone is great, there is no pressure to do more.

2/5 @dlclark file a new issue https://github.com/mattermost/mattermost-plugin-jira/issues/new, it's a great starter ticket for someone else? And you'll get the pleasure of being notified when someone uses the code you committed? :)

Currently making an attempt to use code from this PR here. So far so good :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

demedos picture demedos  路  37Comments

esethna picture esethna  路  36Comments

proximiteclient picture proximiteclient  路  36Comments

ArchangeGabriel picture ArchangeGabriel  路  55Comments

esethna picture esethna  路  28Comments