andreigurau commented on code in PR #24197:
URL: https://github.com/apache/beam/pull/24197#discussion_r1024133728
##########
sdks/java/io/splunk/src/main/java/org/apache/beam/sdk/io/splunk/SplunkEventWriter.java:
##########
@@ -134,12 +156,36 @@ public void setup() {
LOG.info("Disable certificate validation set to: {}", disableValidation);
}
+ // Either user supplied or default enableBatchLogs.
+ if (enableBatchLogs == null) {
+
+ if (enableBatchLogs() != null) {
+ enableBatchLogs = enableBatchLogs().get();
+ }
+
+ enableBatchLogs = MoreObjects.firstNonNull(enableBatchLogs,
DEFAULT_ENABLE_BATCH_LOGS);
+ LOG.info("Enable Batch logs set to: {}", enableBatchLogs);
+ }
+
+ // Either user supplied or default enableGzipHttpCompression.
+ if (enableGzipHttpCompression == null) {
+
+ if (enableGzipHttpCompression() != null) {
Review Comment:
Didn't make any changes here, but I don't think there's a way to combine the
two statements to keep it logically the same.
Eg. Having a statement like
```
enableGzipHttpCompression =
MoreObjects.firstNonNull(enableGzipHttpCompression().get(),
DEFAULT_ENABLE_GZIP_HTTP_COMPRESSION);
```
outside the `if (enableGzipHttpCompression() != null)` block will throw a
NullPointerException if `enableGzipHttpCompression()` is null and having that
above statement inside the if block can cause `enableGzipHttpCompression` to
not be set to `DEFAULT_ENABLE_GZIP_HTTP_COMPRESSION` if
`enableGzipHttpCompression()` is null.
Plus, all the other options, including the ones that existed before these
changes, all follow the same structure of setting the option's variable in this
way
--
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]
