AynSiddiqui opened a new pull request, #2492: URL: https://github.com/apache/iceberg-go/pull/2492
Refs #2443 ## What - Read `idempotency-key-lifetime` from `GET /v1/config`. When it is a non-null string, send a fresh UUIDv7 `Idempotency-Key` on create/drop namespace, update namespace properties, create/register/update/commit/drop/purge/rename table, commit transaction, replace/drop/rename/register view, and cancel planning. Both `UpdateTable` and `CommitTable` are covered. - Do not generate keys for reads, create view or the config call. Absent or null lifetime disables automatic keys on the mutation calls listed above. Generated keys override a configured default `Idempotency-Key` on advertised covered mutations; default headers otherwise retain their existing behavior. ## Why The REST spec lets a server advertise idempotency support so clients can resend a mutation that may already have been applied. Java has sent the header since apache/iceberg#14740; the corresponding PyIceberg PR, apache/iceberg-python#4310, is open. ## Scope This is the first of two PRs, following Java's sequence: apache/iceberg#14740 for headers, then apache/iceberg#17947 and apache/iceberg#18070 for retries and lifetime bounds. This PR adds no explicit HTTP-status retry loop. The follow-up will add retries on retriable status codes, bound them by the advertised lifetime, and reconcile scan plan/task handling. Those POSTs already generate and retry keys independently of lifetime advertisement and are unchanged here. With the header set, Go's standard `net/http` transport may replay certain failed requests on reused connections with the same key. The advertised lifetime is not yet enforced, including `PT0S` or malformed values. This behavior is documented in `website/src/configuration.md` and `website/src/releases.md`. ## Tests - `TestMutationIdempotencyHeaders`: when advertised, every covered method sends a canonical UUIDv7 and two calls get different keys. The tested reads and create view get no generated key. Absent or null lifetime disables automatic keys on the covered mutations; empty and malformed strings enable them, matching Java's apache/iceberg#14740 presence check. - `TestMutationTransportReplay`: after the server reads the first POST body and drops the reused connection, the advertised and `PT0S` cases resend once with the same key and body. Without advertisement, the client returns an error after one attempt. - `go test -race -count=1 ./catalog/rest/...`, `go test -race -count=20 ./catalog/rest -run '^TestMutationTransportReplay$'`, `golangci-lint run ./catalog/rest/...`, full `make lint`, and `git diff --check origin/main HEAD` pass. Full `make test`, integration tests and `dev/check-api-compat.sh` weren't run; `apidiff` isn't installed locally. No exported API changed. ## Questions - Is this split OK, or would you prefer retries and lifetime bounds in the same PR? - The spec does not declare the header on create view, but Java's apache/iceberg#14740 and the proposed PyIceberg change in apache/iceberg-python#4310 include it there. Should Go follow the spec's endpoint list or match those implementations? -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
