ganesh-sadanala commented on code in PR #16169:
URL: https://github.com/apache/kafka/pull/16169#discussion_r1623637352
##########
clients/src/test/java/org/apache/kafka/common/network/DefaultChannelMetadataRegistry.java:
##########
@@ -22,9 +22,10 @@ public class DefaultChannelMetadataRegistry implements
ChannelMetadataRegistry {
@Override
public void registerCipherInformation(final CipherInformation
cipherInformation) {
- if (this.cipherInformation != null) {
- this.cipherInformation = cipherInformation;
+ if (cipherInformation == null) {
+ throw new IllegalArgumentException("cipherInformation cannot be
null");
Review Comment:
I see that `SelectorChannelMetadataRegistry#registerCipherInformation` does
not throw any such exception. The proposed change for
`DefaultChannelMetadataRegistry` is because it is only used in test classes,
not for production. However, I see that there are no invocations of
`DefaultChannelMetadataRegistry#registerCipherInformation` so far. In my
opinion to keep it consistent, either both the classes should have exception
thrown and handled properly, otherwise since
`SelectorChannelMetadataRegistry#registerCipherInformation` is currently used
in production and it does not have the exception thrown,
`DefaultChannelMetadataRegistry#registerCipherInformation` does not require any
exception needed. @divijvaidya @dajac You want to add anything? I guess the
author suggested to throw the exception for future use.
--
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]
