danielcweeks commented on code in PR #16822:
URL: https://github.com/apache/iceberg/pull/16822#discussion_r3508877331


##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3680,9 +3680,16 @@ components:
 
         ## Remote Signing
 
-        If remote signing for a specific storage provider is enabled, clients 
must respect the following configurations when creating a remote signer client:
-         - `signer.endpoint`: the remote signer endpoint. Required. Can either 
be a relative path (to be resolved against `signer.uri`) or an absolute URI.
-         - `signer.uri`: the base URI to resolve `signer.endpoint` against. 
Optional. Only meaningful if `signer.endpoint` is a relative path. Defaults to 
the catalog's base URI if not set.
+        If remote signing for a specific storage provider is enabled, the 
server SHOULD use the `remote-signing-config`
+        field to communicate all signer client settings. When the 
`remote-signing-config` field is present, clients
+        SHOULD take it into account.
+
+        For backward compatibility, the following `config` properties are 
still supported but **DEPRECATED**:
+         - `signer.endpoint` **DEPRECATED**.: the remote signer endpoint. Can 
either be a relative path (to be resolved against `signer.uri`) or an absolute 
URI.
+         - `signer.uri` **DEPRECATED**.: the base URI to resolve 
`signer.endpoint` against. Only meaningful if `signer.endpoint` is a relative 
path. Defaults to the catalog's base URI if not set.
+        If any of these properties is present, clients MUST use them to 
compute the actual remote signing endpoint URI to contact.
+        If none of these properties is present, clients MUST contact the 
default remote signing endpoint using the catalog's base URI.

Review Comment:
   ```suggestion
           If any of these properties is present, clients SHOULD use them to 
compute the actual remote signing endpoint URI to contact.
           If none of these properties is present, clients SHOULD contact the 
default remote signing endpoint using the catalog's base URI.
   ```
   
   Since we've already added the sign endpoint, we should loosen this since 
it's possible (though unlikely) that someone is already redirecting to those 
endpoints automatically when present in the endpoints config.  We should 
shouldn't add strong requirements that we're explicit before.



-- 
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]

Reply via email to