Extract livekit-signaling (and an unexpected side effect, livekit-region) from livekit-api - #1345
Extract livekit-signaling (and an unexpected side effect, livekit-region) from livekit-api#13451egoman wants to merge 11 commits into
livekit-signaling (and an unexpected side effect, livekit-region) from livekit-api#1345Conversation
Moves access_token.rs, jwt_provider.rs and the test_token.txt fixture, plus the get_env_keys helper, into a new livekit-token crate. livekit-api re-exports it as `access_token`, which stays the documented path, so no consumer changes. Also fixes services-tokio and services-async, which used the access-token types without declaring the access-token feature: building with --no-default-features --features services-tokio failed to compile. The extraction forces the fix, since services/ can no longer reach the types any other way. Adds livekit-api/tests/backcompat_paths.rs, a compile-only guard on the re-export paths, since nothing else in the workspace imports them. Also records in the implementation plan that test_token.txt has to move with the token code; the plan had missed it, and cargo test -p livekit-token fails without it.
region.rs was the only module shared by both sides of the upcoming livekit-api split: the signalling region provider and the server-API failover path. Moving it to its own crate lets livekit-signaling and livekit-api each depend on it without either depending on the other. Pure code motion: region was never public, so there is no compatibility shim and no public API change.
The rebase left livekit-token declared twice in livekit-api's [dependencies] and two identical [packages.livekit-token] tables in knope.toml. The former broke the workspace outright: cargo metadata failed with "duplicate key" at livekit-api/Cargo.toml:77.
Moves signal_client/ into its own crate. mod.rs becomes lib.rs, so the
items land at the crate root and livekit-api's historical
`signal_client::` paths are restored by a module wrapping a glob
re-export.
That shim is marked #[deprecated] and #[doc(hidden)]: unlike
access_token, this path is not supported API. Note the deprecation only
fires on `use livekit_api::signal_client;`, not on
`use livekit_api::signal_client::{Item}` — rustc lints a deprecated
module only when it is the final path segment — so existing dependents,
including livekit, keep building without new warnings.
Feature resolution is unchanged: livekit-signaling/native-<rt> forwards
to livekit-net/native-<rt>, whose own definition already folds in
livekit-runtime/<rt>.
livekit-api no longer depends on livekit-net, livekit-common, os_info,
device-info, flate2 or parking_lot, and drops two dependencies that were
declared but never used: scopeguard and bytes.
Two edits the moved code did need: four pub(super) items became
pub(crate), since lib.rs is now the crate root and super would go above
it; and nine tests gated on livekit-api's `signal-client-tokio` feature
were retargeted to livekit-signaling's `tokio`, without which they were
silently compiled out.
Changeset ✓This PR includes a changeset covering all affected packages:
|
| @@ -40,10 +44,10 @@ jobs: | |||
| cmd: cargo test -p livekit-api --no-default-features --features services-tokio,access-token --lib services::api_test -- --nocapture | |||
| - name: services (async / isahc) | |||
| cmd: cargo test -p livekit-api --no-default-features --features services-async,access-token --test services_async -- --nocapture | |||
| - name: signal-client (tokio) | |||
| cmd: cargo test -p livekit-api --no-default-features --features signal-client-tokio --lib signal_client -- --nocapture | |||
| - name: signal-client (async) | |||
| cmd: cargo test -p livekit-api --no-default-features --features signal-client-async --lib signal_client -- --nocapture | |||
| - name: livekit-signaling (tokio) | |||
| cmd: cargo test -p livekit-signaling --no-default-features --features tokio -- --nocapture | |||
| - name: livekit-signaling (async) | |||
| cmd: cargo test -p livekit-signaling --no-default-features --features async -- --nocapture | |||
| services: | |||
| mock-server: | |||
There was a problem hiding this comment.
I'm not convinced this is a good idea - a LLM did this. Should there instead be a livekit-signaling specific ci job instead? Or maybe there's an argument I should keep things as they are and let lukas take over refining this as part of the signaling work more generally.
There was a problem hiding this comment.
🟡 New crate does not build with default features
The library imports livekit_runtime::{interval, sleep, Instant, JoinHandle} unconditionally, but the crate's default = [] selects no runtime flavour, so livekit_runtime re-exports nothing. cargo build -p livekit-signaling with defaults and cargo publish's verification build both fail with unresolved imports.
(Refers to this code)
Prompt for agents
livekit-signaling/src/lib.rs unconditionally imports items from livekit_runtime (interval, sleep, Instant, JoinHandle), but livekit-signaling/Cargo.toml declares default = [] and none of its dependencies enable a runtime flavour by default. livekit-runtime only exports these items behind its tokio/async/dispatcher features, so a default-feature build resolves to an empty livekit_runtime and fails with unresolved imports. This means a bare `cargo build -p livekit-signaling` and, importantly, `cargo publish`'s default-feature verification build will fail (blocking the planned crates.io publish). Decide on the intended default: either pick a sensible default runtime (e.g. default = ["tokio"], matching how the crate is normally consumed) or add an explicit compile_error! guard when no runtime feature is selected so the failure is a clear diagnostic rather than cryptic unresolved-import errors. Note that livekit-net solved the same problem by making livekit-runtime optional and cfg-gating its usage.
Was this helpful? React with 👍 or 👎 to provide feedback.
livekit-signaling (and an unexpected eide effect, livekit-region) from livekit-apilivekit-signaling (and an unexpected side effect, livekit-region) from livekit-api
Project Overview
Right now,
livekit-apiis a crate that is an amalgamation of many things:We've discussed on the client team that it would be good to start to split this up. Larger, monolithic crates are harder to build and require larger numbers of features (which can interact in unexpected ways) than smaller ones, and ideally once this migration is complete,
livekitwill be able to depend on a handful of relevant smaller constituent crates rather than the monolithiclivekit-api.Description
This pull request builds on #1344 and extracts out a new
livekit-signalingcrate fromlivekit-api, which contains all signaling code. Arguably this being inlivekit-apiin the first place (givenlivekit-apiis meant to be "serverside apis") was a little weird, though that was where the http infrastructure lived so I guess it made a certain amount of sense.Now though, all the client focused http infrastructure has been moved to
livekit-netin #1258, so there's very little forcing them to be in the same crate.One small hiccup I did encounter: both existing serverside apis and signaling code both depend on region selection code, so in order to break out signaling, this needs to get moved to a "third thing" - I briefly considered
livekit-common, but aslivekit-apiandlivekit-signalingare likely to be the only dependents, I made a newlivekit-regioncrate which I relocated this region selection code into.This pull request will likely conflict with the signaling refactoring / state machine work lukas is working on. The actual code changes here are a "lift and shift", so lukas's updates should be able to be rebased on top of this into place without a ton of effort.
TODO
livekit-signalingandlivekit-regioncrates to crates.io