-
Notifications
You must be signed in to change notification settings - Fork 71
Modify socket path to conform to FHS #257
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
Conversation
Unix domain sockets should be in /run/. Signed-off-by: Hugues de Valon <[email protected]>
Do we need to modify and upstream the client first? |
Yes probably, for the CI to pass. I have created a PR on the client here. I also realised I forgot pushing updates to the systemd unit file, I am doing that now. |
ccd293e
to
d8927d4
Compare
Signed-off-by: Hugues de Valon <[email protected]>
d8927d4
to
3e41670
Compare
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.
Looks good to me. Hope it won't come to haunt us 🤞
@@ -17,6 +17,7 @@ | |||
|
|||
# Log level to be applied across the service. Can be overwritten for certain modules which have the same | |||
# configuration key. Possible values: "debug", "info", "warn", "error", "trace" | |||
# WARNING: This option will not be updated if the configuration is reloaded with a different one. |
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.
Nice, thanks!
|
||
let listener = UnixListener::bind(SOCKET_PATH)?; | ||
// Will fail if a file already exists at the 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.
Then why don't we just panic
instead of error
on line 54?
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 think this is in the same line of ideas of trying to avoid as many panic as possible 😃
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.
True, I guess we can let it wind down, though I wouldn't mind panics for the issues with which the service cannot continue worrking
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.
👍 Thanks!
Signed-off-by: Hugues de Valon <[email protected]>
Unix domain sockets should be in /run/.
See #250 for reference.