adutra commented on code in PR #15112:
URL: https://github.com/apache/iceberg/pull/15112#discussion_r2725713252
##########
aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java:
##########
@@ -94,13 +112,15 @@ public Supplier<Map<String, String>>
requestPropertiesSupplier() {
@Value.Lazy
public String baseSignerUri() {
- return properties().getOrDefault(S3_SIGNER_URI,
properties().get(CatalogProperties.URI));
+ return properties()
+ .getOrDefault(CatalogProperties.SIGNER_URI,
properties().get(CatalogProperties.URI));
Review Comment:
I don't think so, since the new client introduced in 1.11 is resilient to
older servers.
Let's break this into concrete situations:
<table>
<tr><th>Server</th><th>Client</th><th>Signer config</th><th>Resulting
URI</th><th>Result</th></tr>
<tr><td>1.10</td><td>1.11</td><td>s3.signer.uri=...<br/>s3.signer.endpoint=...</td><td>s3.signer.uri
+ s3.signer.endpoint</td><td>✅ </td></tr>
<tr><td>1.10</td><td>1.11</td><td>s3.signer.uri=...</td><td>s3.signer.uri +
default endpoint</td><td>✅</td></tr>
<tr><td>1.10</td><td>1.11</td><td>s3.signer.endpoint=...</td><td>catalog URI
+ s3.signer.endpoint</td><td>✅</td></tr>
<tr><td>1.10</td><td>1.11</td><td>(empty)</td><td>catalog URI + default
endpoint</td><td>✅</td></tr>
<tr><td>1.11</td><td>1.10</td><td>signer.uri=...<br/>signer.endpoint=...<br/>s3.signer.uri=...<br/>s3.signer.endpoint=...</td><td>s3.signer.uri
+ s3.signer.endpoint</td><td>✅</td></tr>
<tr><td>1.11</td><td>1.10</td><td>signer.endpoint=...<br/>s3.signer.endpoint=...</td><td>catalog
URI + s3.signer.endpoint</td><td>✅</td></tr>
<tr><td>1.11</td><td>1.10</td><td>signer.uri=...<br/>signer.endpoint=...</td><td>catalog
URI + default endpoint</td><td>❌</td></tr>
<tr><td>1.11</td><td>1.10</td><td>signer.endpoint=...</td><td>catalog URI +
default endpoint</td><td>❌</td></tr>
</table>
So in summary:
* 1.11 Clients won't break older servers.
* 1.11 Servers won't break older clients _iif_ they include both `signer.*`
and `s3.signer.*` properties for backwards compatibility (which they all should
do).
However, once support for the deprecated `s3.signer.*` properties is removed
(1.12), _newer clients would break older servers (<= 1.10)_. If that's
concerning, we could e.g. wait a few more minor releases before removal.
--
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]
