choksi2212 commented on PR #4497: URL: https://github.com/apache/iggy/pull/4497#issuecomment-6129754887
Thanks for the detailed review — all nine comments are addressed in 57ce6bb, with a reply on each thread and a test pinning every behavior change: - **nodelay** now lands via `tcp.WithNoDelay`, with the client default (Nagle off) preserved when the option is absent. Observable through the new `GetNoDelay()` accessor. - **Zero spellings** are rejected for `reconnection_interval` and `heartbeat_interval` (non-zero durations in the Rust SDK) and accepted for `reestablish_after` (plain duration), exactly as the Rust typing splits them. - **Duration units** now match the Node and C# SDKs' table (`IggyDurationParser.cs`): `hr`, `hrs`, `wk`, `wks`, `month`, `months` added, `millis` in, `milliseconds` out, with exact-fraction math so `1.005s` parses and `0.5ns` is refused. - **Transport errors** echo only an ASCII-letter transport name, so a malformed scheme with embedded credentials cannot leak them. - **Bad port** returns `InvalidConnectionString`, so `errors.Is` covers the whole parse. - Test renamed to `ParsesBracketedIPv6Host`; the rejection table is attributed to the Node SDK; the unused `frameOffsetClient` is deleted. - The frame offsets, fake server, reply builders and self-signed-certificate helper moved into one shared internal test-support package, `foreign/go/internal/fakevsr`, imported by both test binaries. Verified locally: `go test ./client/... -race` green (client 3.4s, tcp 24s), `go vet ./...` clean, gofmt clean, `go mod tidy` a no-op. One ask: the earlier **BDD job failure on this PR was an infra flake, not the diff** — the docker build got an HTTP 500 fetching `cargo-about-0.9.0` from GitHub releases (the URL returns 200 now, another PR's Pre-merge failed the same minute, and two more went green minutes later). I can't re-run workflows from a fork — could you re-run the BDD job on the new head? Everything else (Pre-merge, PR Title) is waiting on the same first-time-fork approval gate. -- 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]
