-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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/snappy cmd #127
Feat/snappy cmd #127
Conversation
if err != nil { | ||
return err | ||
} | ||
lk, err := lock.Lock(confdir + "/daemon.lock") |
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.
minor thing: "/daemon.lock" -> const daemonLock = "/daemon.lock"
8a02252
to
1b0ec33
Compare
lk, err := daemonLock(confdir) | ||
if err == nil { | ||
lk.Close() | ||
return ErrDaemonNotRunning |
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 changed the order here, the lk.Close()
was unreachable @whyrusleeping
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.
Ah, thanks!
I rebased this on top of new master. daemon_test is not there yet. Aside from an interface change, the test fails (looks like sync issues on close-- DaemonListener's |
Fixed the test now. This still needs to be rebased on top of new master (conflicts will happen). |
**For now**, we don't need to load/parse the private key (which causes a signficant delay in commands) when doing things entirely offline. This may change, and in that case the private key should be loaded on demand.
Erroring out in core setup should cancel the context to ensure subsystems are shut down. This has to happen all over the place we use contexts. @perfmode @whyrusleeping
cbf2b28
to
7492a70
Compare
Okay, rebased on top of master resolving conflicts. :) shipping this. |
👍 |
misc test improvements
Lets get these changes reviewed and merged in, makes things a bit more... "snappy"