-
Notifications
You must be signed in to change notification settings - Fork 428
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
Remove per-operation Open()
/Close()
calls to BoltDB
#316
Conversation
@@ -410,6 +410,8 @@ func (cmd *indexCmd) Run(args []string) { | |||
byId := *cmd.byId | |||
byMatches := *cmd.matches | |||
sources, context, path := preprocessArgsByToggle(args, byMatches || byId) | |||
context.OpenDB() |
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.
We'll need to handle at least OpenDB() errors, please set it to
exitWithError(context.OpenDB())
I've added a few comments. --git a/cmd/drive/main.go b/cmd/drive/main.go
index b417f20..f43d837 100644
--- a/cmd/drive/main.go
+++ b/cmd/drive/main.go
@@ -411,6 +411,9 @@ func (cmd *indexCmd) Run(args []string) {
byMatches := *cmd.matches
sources, context, path := preprocessArgsByToggle(args, byMatches || byId)
+ exitWithError(context.OpenDB())
+ defer context.CloseDB()
+
options := &drive.Options{
Sources: sources,
Hidden: *cmd.hidden, |
I have made the suggested changes @odeke-em! Please take a look at it again. |
} | ||
|
||
func (c *Context) CloseDB() error { | ||
return c.DB.Close() |
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.
if c.DB == nil {
return errors.New("nil dereference of db")
}
return c.DB.Close()
Thanks @js-ojus, just one more and then we are good to go. |
Once that is in, please help me squash the commits into one and then I'll merge it in. |
* `Context` now maintains a DB handle that is kept alive
Hello @odeke-em! I have squashed the commits into a single one, which is now reflected in the pull request. |
LGTM! Thank you very much for diagnosing, and working on this. |
Context
now maintains a DB handle that is kept alive