smiklosovic commented on code in PR #3985:
URL: https://github.com/apache/cassandra/pull/3985#discussion_r2030236168
##########
src/java/org/apache/cassandra/net/InboundConnectionSettings.java:
##########
@@ -146,7 +147,7 @@ public InboundConnectionSettings
withLegacySslStoragePortDefaults()
ServerEncryptionOptions encryption = this.encryption;
if (encryption == null)
encryption =
DatabaseDescriptor.getInternodeMessagingEncyptionOptions();
- encryption =
encryption.withOptional(false).withInternodeEncryption(ServerEncryptionOptions.InternodeEncryption.all);
+ encryption = new
Builder(encryption).withOptional(false).withInternodeEncryption(ServerEncryptionOptions.InternodeEncryption.all).build();
Review Comment:
by using `ServerEncryptionOptions.builder(encryption)`, we at least know
that we are building server encryption options. When you do `new Builder(...)`
then we are building _what_ exactly?
You are already using `new EncryptionOptions.Builder(encOptions);` which is
fine. I would just remove import of `ServerEncryptionOptions.Builder` and do it
like `new ServerEncryptionOptions.Builder(encryption)` to match how it is done
elsewhere.
##########
src/java/org/apache/cassandra/net/InboundConnectionSettings.java:
##########
@@ -146,7 +147,7 @@ public InboundConnectionSettings
withLegacySslStoragePortDefaults()
ServerEncryptionOptions encryption = this.encryption;
if (encryption == null)
encryption =
DatabaseDescriptor.getInternodeMessagingEncyptionOptions();
- encryption =
encryption.withOptional(false).withInternodeEncryption(ServerEncryptionOptions.InternodeEncryption.all);
+ encryption = new
Builder(encryption).withOptional(false).withInternodeEncryption(ServerEncryptionOptions.InternodeEncryption.all).build();
Review Comment:
@maulin-vasavada by using `ServerEncryptionOptions.builder(encryption)`, we
at least know that we are building server encryption options. When you do `new
Builder(...)` then we are building _what_ exactly?
You are already using `new EncryptionOptions.Builder(encOptions);` which is
fine. I would just remove import of `ServerEncryptionOptions.Builder` and do it
like `new ServerEncryptionOptions.Builder(encryption)` to match how it is done
elsewhere.
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
