adutra commented on PR #8857: URL: https://github.com/apache/iceberg/pull/8857#issuecomment-1823031640
I took the time to investigate what it would take to use the client-side classes consistently. My final opinion is that it's not a good solution, for the below reasons: * Client-side namespace classes do not propagate `NessieConflictException`: they are either [wrapped in `IllegalStateException`](https://github.com/projectnessie/nessie/blob/main/api/client/src/main/java/org/projectnessie/client/api/ns/ClientSideUpdateNamespace.java#L90-L91) or [mixed together with `NessieNotFoundException`](https://github.com/projectnessie/nessie/blob/main/api/client/src/main/java/org/projectnessie/client/api/ns/ClientSideCreateNamespace.java#L108-L109). This limitation comes from the fact that the namespace API in V1 does not throw `NessieConflictException`. This makes it hard for the Iceberg-Nessie client to decide when to retry a commit. * The functionality to add custom commit metas is indeed [present in the API](https://github.com/projectnessie/nessie/blob/main/api/client/src/main/java/org/projectnessie/client/api/ModifyNamespaceBuilder.java#L23), but in reality, [it's not available in V1](https://github.com/projectnessie/nessie/blob/81b34b610897c6f54bca88e8e6d503cfc3bbe05f/api/client/src/main/java/org/projectnessie/client/rest/v1/HttpCreateNamespace.java#L57). This limitation, again, is dictated by the namespace API in V1 which does not allow to set the commit meta. * I further explored the idea of using the client-side namespace classes also for V1, effectively switching V1 clients to use the Tree API under the hood. Although it looks feasible with minor changes at first glance, it becomes quickly a can of worms: many client-side classes use API methods that only exist in V2, such as: `GetContentBuilder#getWithResponse`, `GetMultipleContentsResponse#getEffectiveReference`, and obviously, `CommitMultipleOperationsBuilder#commitWithResponse`. We would need to remove these and use V1-compatible methods instead, but that feels odd now that V1 is deprecated. Per the above, I'd suggest that we keep using the Tree API here. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
