Skip to content

refactor(cli): remove duplicate connection code - #302

Merged
kacy merged 1 commit into
mainfrom
feat/snapshot-scheduling
Feb 25, 2026
Merged

refactor(cli): remove duplicate connection code#302
kacy merged 1 commit into
mainfrom
feat/snapshot-scheduling

Conversation

@kacy

@kacy kacy commented Feb 25, 2026

Copy link
Copy Markdown
Owner

summary

when ember-client was added as a standalone crate, the connection logic
was written fresh rather than wiring the CLI to use it. the result was ~85%
duplication: both crates had their own Connection/Client struct, identical
error types, the same frame read loop, and a copy of TlsClientConfig.

this PR removes the dead copy from ember-cli and wires it to ember-client.

what changed:

  • ember-client::TlsClientConfig is now public and re-exported from the crate root
  • ember-cli::tls re-exports it instead of defining its own identical struct
  • ember-cli gains an ember-client dependency
  • all callers (repl, batch, watch, cluster, oneshot) now use Client::connect /
    Client::connect_tls, .send(), .auth(), and .disconnect()
  • auth_frame in bench_conn.rs was a 4-line, single-caller helper — inlined directly
  • crates/ember-cli/src/connection.rs deleted

bench_conn.rs still owns MaybeTlsStream and the raw TLS connect path for
pipelining — that code is benchmark-specific and stays.

what was tested

  • cargo build -p emberkv-cli — clean, zero warnings
  • cargo clippy -p emberkv-cli -- -D warnings — clean
  • cargo test -p emberkv-cli — 114/114 pass
  • cargo test --workspace — 78/79 integration tests pass; the failing
    cluster_moved_redirect test is a pre-existing flaky race unrelated to this change
  • cargo fmt --all --check — clean

design notes

bench_conn.rs bypasses Client intentionally — it pre-serializes commands
and writes raw bytes for pipeline benchmarking. it still reads from
crate::tls::MaybeTlsStream directly, which is the right level of abstraction
for that use case.

ember-cli had its own connection.rs with ~85% overlap with
ember-client/src/connection.rs — identical error types, frame
read loop, send/auth/disconnect logic, and TlsClientConfig struct.

- expose TlsClientConfig publicly from ember-client (tls feature)
- re-export it from ember-cli::tls, removing the duplicate struct
- add ember-client dependency to ember-cli
- update all callers (repl, batch, watch, cluster, main) to use
  Client::connect / Client::connect_tls, .send(), .auth(), .disconnect()
- inline auth_frame in bench_conn.rs (4-line helper, one call site)
- delete crates/ember-cli/src/connection.rs
@kacy
kacy force-pushed the feat/snapshot-scheduling branch from b5905bb to e354aca Compare February 25, 2026 16:38
@kacy
kacy merged commit 5457052 into main Feb 25, 2026
4 of 7 checks passed
@kacy
kacy deleted the feat/snapshot-scheduling branch February 25, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant