Port over keyhive sync protocol orchestration and wire together#158
Port over keyhive sync protocol orchestration and wire together#158jtfmumm wants to merge 6 commits into
Conversation
jtfmumm
force-pushed
the
jtfm/ark-orchestration
branch
from
f6e7bb0 to
d127df5
Compare
| } | ||
| /// Authorize a put to `sedimentree_id` by `author` against `keyhive`. | ||
| /// | ||
| /// TODO: Only `author` is currently checked for [`Access::Edit`]. |
There was a problem hiding this comment.
We're currently ignoring requestor here. Are we fine with this for the time being?
| @@ -0,0 +1,156 @@ | |||
| //! Custom composed handler that dispatches wire messages to sub-handlers, | |||
| //! forwarding unknown-protocol frames to a registered JS callback. | |||
There was a problem hiding this comment.
The reason this exists right now is that automerge-repo-keyhive is in a transitional state where it connects through the Rust subduction but for the client still uses the TS-side implementation of the keyhive sync protocol. Anything that's not a sync or ephemeral message is handled by its callback, which delegates to the TS protocol.
In the other direction, when automerge-repo-keyhive sends a keyhive message, it calls subduction.sendRawFrame and hands it off.
There was a problem hiding this comment.
Once automerge-repo-keyhive is migrated to use the Rust protocol, I think this goes away.
| "Brooklyn Zelenka <hello@brooklynzelenka.com>" | ||
| ] | ||
|
|
||
| # !@ FIXME: Replace local paths |
There was a problem hiding this comment.
Need to update after keyhive release
jtfmumm
force-pushed
the
jtfm/ark-orchestration
branch
3 times, most recently
from
3f97589 to
49b2bea
Compare
jtfmumm
force-pushed
the
jtfm/ark-orchestration
branch
from
b9ad1d9 to
4542ac9
Compare
| let seed = key::load_signer_bytes(&args.key)?; | ||
| let signer = key::signer_from_seed(&seed); | ||
| let keyhive_signer = key::keyhive_signer_from_seed(&seed); | ||
|
|
There was a problem hiding this comment.
Why do this hardcoded inline rather than using the separate type?
| } | ||
| } | ||
|
|
||
| impl KeyhiveConnection<Local> for CliConnKeyhiveAdapter { |
There was a problem hiding this comment.
No Sendable? I need to double check, but I think Keyhive is now both Send and !Send (at your option)
There was a problem hiding this comment.
You're right. I updated for the CLI path, which significantly simplifies things and gets us off that keyhive thread.
| /// | ||
| /// [`on_peer_disconnect`]: subduction_core::handler::Handler::on_peer_disconnect | ||
| pub async fn on_peer_connect(&self, peer_id: PeerId, conn: Conn) { | ||
| drop( |
There was a problem hiding this comment.
will it not drop when the program exits this scope?
There was a problem hiding this comment.
The reason this is here (and in a couple other spots) is that if you do let _ = self.tx.send... you run up against the let-underscore-drop lint. If instead, you just don't drop, you hit unused std::result::Result that must be used. So we're stuck in between let-underscore-drop and unused-must-use. You can actually do something like let _dropped = but that seemed worse than drop(.
What do you think?
jtfmumm
force-pushed
the
jtfm/ark-orchestration
branch
2 times, most recently
from
01938a9 to
ceeeef2
Compare
jtfmumm
force-pushed
the
jtfm/ark-orchestration
branch
from
ceeeef2 to
2313520
Compare
3 participants
This PR mostly ports over the remaining orchestration and periodic caching logic from
automerge-repo-keyhiveand wires this up in place of pre-existing stubs in this repo.This changes the subduction CLI to be keyhive-aware. If we want this to be configurable, I'll need to refactor it (possibly in a future PR).