-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat(orm): add module db #10991
feat(orm): add module db #10991
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10991 +/- ##
==========================================
+ Coverage 65.86% 66.06% +0.20%
==========================================
Files 647 690 +43
Lines 65764 69558 +3794
==========================================
+ Hits 43314 45952 +2638
- Misses 19970 20810 +840
- Partials 2480 2796 +316
|
|
||
// Marshal marshals the provided message to JSON with a stable ordering based | ||
// on ascending field numbers. | ||
func Marshal(message proto.Message) ([]byte, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haven't done a deep check, but maybe there are some protobuf well-known types that need to be json marshalled in a custom way, examples are: timestamp, struct, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you're right... hopefully it's not too bad. will take a look soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for now, maybe this is okay just because the main use case is to do stable marshaling for tests. I'm mainly trying to prevent spurious CI failures rather than a full fledged encoder
// Has returns true if there is an entity in the table with the same | ||
// primary key as message. Other fields besides the primary key fields will not | ||
// be used for retrieval. | ||
Has(ctx context.Context, message proto.Message) (found bool, err error) | ||
|
||
// Get retrieves the message if one exists for the primary key fields | ||
// set on the message. Other fields besides the primary key fields will not | ||
// be used for retrieval. | ||
Get(ctx context.Context, message proto.Message) (found bool, err error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fdymylja added these to the Table
interface as this makes Has
/Get
usage based on the primary key simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the minor changes to the format of an entry that need to be reflected in entry_test.go
, looks good to me. Pre-approving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
Description
This PR adds a
ModuleDB
interface which can be used directly by Cosmos SDK modules. A simplified bank example with Mint/Send/Burn functionality against Balance and Supply tables is included in the tests.This PR also:
Get
andHas
methods toTable
which use the primary key values in the message instead of...interface{}
Entry.String
methods to use it because the golden tests are not deterministic without this. This code is currently internal but can be extracted to a publiccodec
orcosmos-proto
package eventually.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change