TomPlanche opened a new pull request, #4388:
URL: https://github.com/apache/iggy/pull/4388
## Which issue does this PR address?
Relates to #4287
## Rationale
An explicit `disconnect()` must hold until the next `connect()`, as agreed
on #4287: the test in `disconnect_relogin.rs` is the correct contract, not the
`IggyClient` docs. Problems 2 and 3 of #4287 break this rule for a client with
auto-login credentials. This PR fixes these two problems.
## What changed?
After `disconnect()`, a client with auto-login credentials came back on its
own. The heartbeat kept running and its ping reconnected the client and signed
in again, and a direct `ping()` or `get_stats()` did the same. Only an
authenticated request such as `get_me()` failed, because
`fail_if_not_authenticated` rejects it before the transport.
Now the `Client` trait impl of `disconnect()` sets a caller-intent flag on
TCP, QUIC and WebSocket, and the trait impl of `connect()` clears it. While the
flag is set, `send_raw_with_response` fails with `NotConnected` at once: no
packets on the wire, no wait, no reconnect. The flag lives only in the trait
impl, because leader redirection and the reconnect path call the inherent
`disconnect()`. A connection that drops without `disconnect()` leaves the flag
clear, so that loss still heals. `IggyClient::disconnect` also stops the
heartbeat, and `connect` starts it again. This removes the useless pings, and
on QUIC it removes the redial that blocked `disconnect()` in
`endpoint.wait_idle()`.
The `Client` trait now documents the `disconnect()` contract. The
`IggyClient`, Python and C++ docs no longer say that auto-login undoes a
disconnect. The last two lifecycle tests from #4344 lose their `#[ignore]`, so
all 10 cases of `client_lifecycle.rs` now run.
## Local Execution
I used Zed in devcontainer mode. The build and the tests ran in the
devcontainer, and `prek` ran on macOS.
- Passed: `cargo fmt --all` and `cargo clippy -p iggy_common -p iggy -p
integration --all-features --all-targets -- -D warnings`.
- Passed: `cargo test -p iggy --lib`, with 234 tests.
- Passed: the lifecycle tests, with all 10 cases and none ignored, in 3 runs
out of 3:
```text
$ cargo test -p integration -- sdk::client_lifecycle
test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 991 filtered
out
```
- Passed: the `sdk::` integration suite (69 passed, 0 ignored), which
includes `disconnect_relogin.rs`, and the `cluster::` suite (80 passed),
because the flag sits in the reconnect path that failovers use.
- Passed: the Python lifecycle tests against a local server:
```text
$ uv run --no-sync pytest tests/test_connectivity.py -k Lifecycle
16 passed, 35 deselected
```
- Passed: `cargo run --bin stub_gen`. The stub diff contains only the
`disconnect()` docstring.
- Passed: pre-commit hooks with `SKIP=web-lint prek run --all-files`. I
skipped `web-lint` because this PR does not change `web/`.
## AI Usage
1. None for the code, tried to write this PR body with Claude but finished
with some 200+ lines slop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
