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

new: manipbolt #175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

new: manipbolt #175

wants to merge 1 commit into from

Conversation

sibicramesh
Copy link
Contributor

Created by: sibicramesh

manipbolt implements a schema less persistent storage backed by boltdb+storm. It implements manipulate interface.

To begin

To create a new manipbolt

m, err := New(smc.db, gaia.Manager())

Thats it!

Features

and much more.

Error handling

  • All errors returned by the package is of type manipulate.Error. The underlying error is part of the Error() method`.

Unit tests

Unit test coverage is ~95%.


Inspired by

https://github.com/asdine/storm
https://github.com/etcd-io/bbolt
https://github.com/aporeto-inc/manipulate

@sibicramesh sibicramesh self-assigned this Mar 16, 2023
@sibicramesh
Copy link
Contributor Author

/build

Copy link
Contributor

@primalmotion primalmotion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! That's promising. It will be easier to work by batch review.
The first review is about the filter compiler

@@ -3,41 +3,50 @@ module go.aporeto.io/manipulate
go 1.20

require (
go.aporeto.io/elemental v1.122.0
go.aporeto.io/wsc v1.51.0
go.aporeto.io/elemental v1.122.1-0.20230127211758-51fb47178716
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please stay on releases

go.aporeto.io/elemental v1.122.0
go.aporeto.io/wsc v1.51.0
go.aporeto.io/elemental v1.122.1-0.20230127211758-51fb47178716
go.aporeto.io/wsc v1.51.1-0.20230127211509-870916470c0c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please stay on releases

return &config{}
}

// OptionCodec used to set a custom encoder and decoder.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// OptionCodec used to set a custom encoder and decoder.
// OptionCodec sets a custom encoder and decoder.


case elemental.EqualComparator:

items = append(items, containsOrEqual(k, values[0]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't checked that function yet, but either it is not doing what it says it does (equality check) or should not be used. From what I understand, a == "y" would match x if x == "ay"

subs = append(subs, regexMatcher(k, v))
}

items = append(items, q.Or(subs...))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check or is the behavior of other filter compilers.


items = append(items, q.Or(subs...))

case elemental.ContainComparator:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should also be the behavior for elemental.InComparator

}

return q.And(matchers...), nil
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this misses other basic comparators, namely

  • elemental.NotEqualComparator
  • elemental.NotContainComparator
  • elemental.NotInComparator
  • elemental.GreaterComparator
  • elemental.GreaterOrEqualComparator
  • elemental.LesserComparator
  • elemental.GreaterOrEqualComparator
  • elemental.ExistsComparator
  • elemental.NotExistsComparator

{
name: "invalid filter",
args: args{
elemental.NewFilterComposer().WithKey("pid").NotEquals("5d83e7eedb40280001887565").Done(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should test those against the String() version of the filter

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 this pull request may close these issues.

2 participants