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 GET and LIST APIs for retrieving registered patterns #73

Open
embano1 opened this issue Jun 20, 2022 · 4 comments
Open

Add GET and LIST APIs for retrieving registered patterns #73

embano1 opened this issue Jun 20, 2022 · 4 comments

Comments

@embano1
Copy link
Collaborator

embano1 commented Jun 20, 2022

Currently in quamina when using the default matcher, i.e. quamina.New() there is no way to retrieve the registered patterns, e.g. for debugging, inspection, pattern handling (review/replace during runtime, etc.).

The following APIs would be useful (just a proposal, need to figure return types):

  • func (q *Quamina) GetPattern(x X) (string, error) (note: assumes pattern is of type string)
  • func (q *Quamina) ListPatterns() ([]string, error) (note: assumes pattern is of type string)
@timbray
Copy link
Owner

timbray commented Jun 20, 2022

Good idea, but remember that you can say

q.AddPattern(`{"a": [1]}` )
q.AddPattern(`{"a":{"b": ["33"]}}`

I.e. multiple patterns for the same X. So the first API needs to be something like:

func (q *Quamina) GetPatternsFor(x X) ([]string, error)

I'm trying to think of a use-case for the second proposal and coming up empty so far. Nothing wrong with the proposal though.

Finally, my experience suggests anything that returns a plural result will probably eventually need pagination tokens.

@embano1
Copy link
Collaborator Author

embano1 commented Jun 20, 2022

Good idea, but remember that you can say

q.AddPattern(`{"a": [1]}` )

q.AddPattern(`{"a":{"b": ["33"]}}`

I.e. multiple patterns for the same X. So the first API needs to be something like:

func (q *Quamina) GetPatternsFor(x X) ([]string, error)

Good point (multiple patterns). IMHO GetPatterns(x X) (or even just Patterns(x X)) still works as name bc it's pretty clear even without For, just needs []string as you suggest.

I'm trying to think of a use-case for the second proposal and coming up empty so far. Nothing wrong with the proposal though.

Finally, my experience suggests anything that returns a plural result will probably eventually need pagination tokens.

All fair points, let's defer List for now then.

@timbray
Copy link
Owner

timbray commented Jun 20, 2022

Hey @jsmorph is there anything about this that would be hard?

@jsmorph
Copy link
Collaborator

jsmorph commented Jun 21, 2022

The Pruner has LivePatternState, which offers something close. The standard implementation is memState, which really is just a map[X]stringSet. GetPattern(X) ([]string, error) would be fine to add, I think.

Re ListPatterns: I personally prefer a function like func(func(x X, pattern string) error) error in order to reduce allocation and/or avoid pagination. LivePatternState has Iterate(func(x X, pattern string) error) error).

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