Skip to content
Please note that GitHub no longer supports Internet Explorer.

We recommend upgrading to the latest Microsoft Edge, Google Chrome, or Firefox.

Learn more
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

activation: add logic to receive file-descriptors #30

Merged
merged 4 commits into from Nov 16, 2019

Conversation

@jabedude
Copy link
Contributor

jabedude commented Nov 13, 2019

Initial PR for #29, please review and let me know what you think.

src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
src/traits.rs Outdated Show resolved Hide resolved
@jabedude jabedude force-pushed the jabedude:socket-activate branch from 3b7c6bf to 0d42fd9 Nov 15, 2019
@lucab lucab force-pushed the jabedude:socket-activate branch from d88d395 to e18f478 Nov 15, 2019
src/daemon.rs Outdated Show resolved Hide resolved
src/daemon.rs Outdated Show resolved Hide resolved
@jabedude jabedude force-pushed the jabedude:socket-activate branch from e18f478 to f41ab1e Nov 15, 2019
@lucab

This comment has been minimized.

Copy link
Owner

lucab commented Nov 15, 2019

@jabedude very good!

I think clippy in Travis caught an actual issue with your octal constants. Other than that, I'm fine with merging this.

I only have a few final bikesheddings on naming:

  • what about sticking to systemd convention and put this logic to its own module? I think it could be called activation (like in the Go library) or listen_fds (like in the original C library)
  • we can simply name the functions as receive_descriptors and receive_descriptors_with_names
  • a s/SocketType/FileDescriptor/ is better for the enum, as some of the variants are not really sockets
@lucab

This comment has been minimized.

Copy link
Owner

lucab commented Nov 15, 2019

@jabedude I think we can also freely bump Rust minimum version, it's currently at 1.31.0 for no specific reason.

@jabedude

This comment has been minimized.

Copy link
Contributor Author

jabedude commented Nov 16, 2019

@lucab That's great to hear!

  • I think daemon::activation would be a great idea, I'll do that now

  • That's actually something I've wanted to do as well!

  • Good point, I'll fix that.

@jabedude jabedude force-pushed the jabedude:socket-activate branch 8 times, most recently from 86d3dde to 0c1c0cf Nov 16, 2019
@jabedude jabedude force-pushed the jabedude:socket-activate branch from 0c1c0cf to ef3b145 Nov 16, 2019
@jabedude

This comment has been minimized.

Copy link
Contributor Author

jabedude commented Nov 16, 2019

@lucab I've been fighting the linter and I can't get one check to pass.

@lucab

This comment has been minimized.

Copy link
Owner

lucab commented Nov 16, 2019

@jabedude that required updating Travis config. I've pushed a commit on your branch, CI is green now.

Thanks for the code, this LGTM.

@lucab
lucab approved these changes Nov 16, 2019
@lucab lucab merged commit 25ebae5 into lucab:master Nov 16, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jabedude jabedude deleted the jabedude:socket-activate branch Nov 16, 2019
@lucab lucab changed the title Add sd_listen_fds and sd_listen_fds_with_names activation: add logic to receive file-descriptors Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.