ragen1337 opened a new pull request, #4482:
URL: https://github.com/apache/iggy/pull/4482
## Which issue does this PR address?
Closes #4444
## Rationale
See #4444.
## What changed?
`get_server_address` checked the transport against the string `"websocket"`,
so with `--transport ws` it returned `None` and the CLI panicked on `unwrap()`.
It now parses the transport into `TransportProtocol`, the same way
`ClientProviderConfig` does, so `ws` and `websocket` both work.
`main` now gets the server address once at startup. An unknown transport
like `--transport foo` fails with `InvalidTransport("foo")` instead of a panic,
before any password prompt. The address is passed down to the commands and
`IggyCredentials`, so the five `unwrap()` calls and the `MissingServerAddress`
error are gone.
New integration tests run the five commands over ws, check that `me` reuses
a ws session, and check `--transport foo`. They fail on master.
## Local Execution
- `cargo fmt --all -- --check`, `cargo clippy -p iggy_common -p iggy-cli -p
integration --all-targets --all-features -- -D warnings`, `cargo test -p
iggy_common --lib types::args` and `cargo test -p integration --test mod --
cli::` (169 passed; the two `should_help_match` tests for stream and topic fail
on master too since #4477). Run in a Linux container with gnome-keyring, like
the CI pre-merge action.
- `prek run --from-ref <merge-base> --to-ref HEAD` passed and changed no
files.
## AI Usage
1. Claude Code.
2. It helped with the fix and wrote most of the tests; I went through all of
it.
3. Reproduced the panic on master, then ran the new integration tests
against a local server: they fail on master and pass with the fix. Also ran
fmt, clippy and the CLI test suite.
4. Yes.
--
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]
