-
Notifications
You must be signed in to change notification settings - Fork 4
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
Factor out start-a-libp2p-daemon! #21
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,14 +14,28 @@ | |
(def current-libp2p-daemon | ||
(make-parameter #f)) | ||
|
||
(def (start-libp2p-daemon! host-addresses: (host-addrs #f) daemon: (bin "p2pd") | ||
options: (args []) | ||
address: (sock #f) | ||
wait: (timeo 0.4)) | ||
;; Starts a new libp2p-daemon only if there is no existing current-libp2p-daemon, | ||
;; and if so sets the current-libp2p-daemon to the new one. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe that should be renamed |
||
(def (start-the-libp2p-daemon! host-addresses: (host-addrs #f) daemon: (bin "p2pd") | ||
options: (args []) | ||
address: (sock #f) | ||
wait: (timeo 0.4)) | ||
(cond | ||
((current-libp2p-daemon) | ||
=> values) | ||
(else | ||
(let ((d (start-libp2p-daemon! host-addresses: host-addrs daemon: bin | ||
options: args | ||
address: sock | ||
wait: timeo))) | ||
(current-libp2p-daemon d) | ||
d)))) | ||
|
||
;; Starts a libp2p-daemon without regard to current-libp2p-daemon | ||
(def (start-libp2p-daemon! host-addresses: (host-addrs #f) daemon: (bin "p2pd") | ||
options: (args []) | ||
address: (sock #f) | ||
wait: (timeo 0.4)) | ||
(let* ((path (or sock (string-append "/tmp/p2pd." (number->string (getpid)) ".sock"))) | ||
(addr (string-append "/unix" path)) | ||
(proc (if host-addrs | ||
|
@@ -32,8 +46,7 @@ | |
((process-status proc timeo #f) | ||
=> (lambda (status) | ||
(error "p2pd exited prematurely" status)))) | ||
(current-libp2p-daemon d) | ||
d)))) | ||
d)) | ||
|
||
(def (stop-libp2p-daemon! (d (current-libp2p-daemon))) | ||
(with ((daemon proc path) d) | ||
|
@@ -45,5 +58,9 @@ | |
(delete-file path)) | ||
status)))) | ||
|
||
(def (stop-the-libp2p-daemon!) | ||
(stop-libp2p-daemon!) | ||
(current-libp2p-daemon #f)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't |
||
|
||
(def (use-libp2p-daemon! path) | ||
(current-libp2p-daemon (daemon #f path))) |
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.
Why is it start-a- and stop-the- ?
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 backwards compatibility is a concern,
Current behavior has:
start-libp2p-daemon!
for "the" daemon incurrent-libp2p-daemon
stop-libp2p-daemon!
for "a" daemon, wherecurrent-libp2p-daemon
doesn't matterA better design would have:
start-libp2p-daemon!
for "a" daemonstop-libp2p-daemon!
for "a" daemonstart-the-libp2p-daemon!
for "the" daemon incurrent-libp2p-daemon
stop-the-libp2p-daemon!
for "the" daemon incurrent-libp2p-daemon
The closest thing I could think of to that while maintaining backwards compatibility has:
start-a-libp2p-daemon!
for "a daemon"stop-libp2p-daemon!
for "a" daemon as dictated by existing codestart-libp2p-daemon!
for "the" daemon as dictated by existing codestop-the-libp2p-daemon!
for "the" daemon incurrent-libp2p-daemon
If backwards compatibility is not a concern, I could make it more consistent with only "the" and no "a", but I won't do that unless @vyzo says that's what he wants.
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.
Okay, I've changed the convention to make it more consistent with only "the" and no "a", and also added some documentation.