-
Notifications
You must be signed in to change notification settings - Fork 8
Add the daemon module and the standalone CLI for it #39
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
|
At first I planned to use https://crates.io/crates/jsonrpc, but it turned out to be client-side only. Extending it for server use would require adding several missing features. Other crates on crates.io have similar limitations, or are unnecessarily verbose. Given that the JSON-RPC spec is quite small, I decided to implement a lightweight solution tailored to both our client and server use cases. |
apoelstra
left a comment
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.
ACK 69f5664; successfully ran local tests
Had a previous, longer review but was lost in iteration on local CI setup.
|
Sorry for the noise. I think the "previous, longer review" text was not supposed to be posted publicly, which in turn is confusing my "did you already post this review" logic. Edit Ah, no, it's that my vibe-coded duplicate-detection logic completely fails to handle newlines. |
|
...on that one I forgot to rebuild the runner. |
|
I would like to send some funny gif :D |
Phew, okay, it's working now. The last two were caused by a shell quoting issue, and because I had "if a command fails when checking for dupes, just assume there weren't any" logic. I fixed both. |
|
@ivanlele this PR changes the version from 0.1 to 0.2. Do you want me to cut a new release right after merging? I think we might as well. In these early stages we should cut releases often. |
I believe we already have 0.2, so perhaps the next release should be 0.3 unless I'm missing something? And I agree, we can roll out a new realease, there are a few more approved PRs in the queue that would make a new release even more meaningful |
|
Oh, interesting, this PR doesn't do the release, but because Let me fix that logic and then I'll just merge this. |
One of the PRs in a series addressing issue #28.
This PR introduces a daemon module that exposes a JSON-RPC handler, along with a new binary to start it. To avoid adding a new set of dependencies into the existing binary, a separate daemon feature flag was added.
The daemon can run either as an independent service or be embedded as a library and spawned as a separate thread within another process.