-
Notifications
You must be signed in to change notification settings - Fork 33
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
[Persistence] Update KVStore to use Badger wrapper functions #838
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #838 +/- ##
==========================================
+ Coverage 31.52% 31.86% +0.33%
==========================================
Files 107 108 +1
Lines 9034 9126 +92
==========================================
+ Hits 2848 2908 +60
- Misses 5846 5874 +28
- Partials 340 344 +4
☔ View full report in Codecov by Sentry. |
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.
PTAL at couple tiny comments and feel free to merge after
key: invalidKey[:], | ||
value: []byte("bar"), | ||
fail: true, | ||
expected: errors.Errorf("Key with size 65001 exceeded 65000 limit. Key:\n%s", hex.Dump(invalidKey[:1<<10])), |
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.
hex.Dump(invalidKey[:1<<10])),
🆒
NOTE @Olshansk, @dylanlott is currently working on savepoints/rollbacks that touches the same code and this PR will probably only be used as a reference as he is changing the logic their. The new wrapper functions fix the bug with |
This PR is already complete and ready to be merged in, while savepoints & rollbacks PR is 1+ weeks away. IMO we should merge this in and iterate on top of it. However, I will defer to @dylanlott to decide. |
I agree with merging this and addressing the logic changes necessary for savepoints later. It shouldn't hold this up. |
Description
This PR introduces unit tests to cover the KVStore's functionality as well as updating the KVStore logic to use the Badger wrapper functions
update
,view
, etc.This fixes the issues around deleting keys from the KVStore.
Issue
Fixes N/A
Type of change
Please mark the relevant option(s):
List of changes
Testing
make develop_test
; if any code changes were mademake test_e2e
on k8s LocalNet; if any code changes were madee2e-devnet-test
passes tests on DevNet; if any code was changedRequired Checklist
godoc
format comments on touched members (see: tip.golang.org/doc/comment)If Applicable Checklist
shared/docs/*
if I updatedshared/*
README(s)