neils-dev commented on code in PR #3297:
URL: https://github.com/apache/ozone/pull/3297#discussion_r852514357
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1354,9 +1354,9 @@ message UpdateGetS3SecretRequest {
This will be used by OM to authenticate S3 gateway requests on a per request
basis.
*/
message S3Authentication {
- optional string stringToSign = 1;
- optional string signature = 2;
- optional string accessId = 3;
+ required string stringToSign = 1;
+ required string signature = 2;
+ required string accessId = 3;
Review Comment:
The S3Authentication message itself is _optional_ in the OMRequest.
https://github.com/apache/ozone/blob/a2a27025ee37d5b3ad4f47e3cc37f92fc4a86073/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto#L188
Only if the S3Authentication field (optional) is within the OMRequest that
the message fields (stringToSign, etc) are required.
Is it better, to have the message fields optional even when the presence of
the S3Authentication field is optional in the OMRequest? Or ensure, require
that all the fields are present when an S3Authentication is added to an
OMRequest?
--
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]
