-
Notifications
You must be signed in to change notification settings - Fork 53
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
fix: Ignore badger path if in-memory #2967
fix: Ignore badger path if in-memory #2967
Conversation
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.
@ONLYUSEmePHONE Hey, and welcome to our codebase!
Thank you very much for this fix, it solves the bigger problem, and is very minimal as you noted in the description :)
…ory is not true when creating a new badger datastore
72aa29e
to
9c9a772
Compare
FYI: I'm going to update the PR title, hope you don't mind - we have a 60 character limit (I also rebased onto the latest, also a requirement for merge) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2967 +/- ##
===========================================
+ Coverage 79.34% 79.35% +0.01%
===========================================
Files 326 326
Lines 24768 24769 +1
===========================================
+ Hits 19651 19655 +4
+ Misses 3704 3702 -2
+ Partials 1413 1412 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 14 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Relevant issue(s)
Resolves #2964
Description
Small change that now only sets the badger options
Dir
andValueDir
to thepath
parameter if the badger optionInMemory
is nottrue
.The crash was happening because badger expects the two Dir values to be nil if InMemory is true.
I chose this fix because it's very simple and requires the least changes. The bulk of the problem is that the
datastore.badger.path
config value gets set todata
by default regardless of whetherdatastore.store
is set tomemory
and this value gets passed as thepath
parameter. I didn't think it would be nice to change any of the upstream logic or the signature of the NewDatastore function.Tasks
How has this been tested?
Running standard test suite and using both commands provided in #2964 .
Specify the platform(s) on which this was tested: