adutra commented on code in PR #15850:
URL: https://github.com/apache/iceberg/pull/15850#discussion_r3033195604


##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3522,6 +3522,11 @@ components:
         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.
+         - `signer.properties.*`: additional properties to be passed through 
to the signer endpoint in remote sign
+           requests. Optional. If such properties are present, signer clients 
MUST pass them through to the signer

Review Comment:
   1. I think these properties are being given excessive meaning; they should 
remain entirely opaque from the client's standpoint. Defining them as mandatory 
would significantly improve the clarity of the specification. Relying on a 
"SHOULD" is problematic, as it essentially informs clients that while sending 
these properties is recommended, omitting them is also acceptable, which 
strikes me as a poor specification.
   2. Again, this topic is outside of scope for this PR imho. I suggest we 
resume this conversation when implementing this feature in Apache Polaris.



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