gemini-code-assist[bot] commented on code in PR #38588:
URL: https://github.com/apache/beam/pull/38588#discussion_r3282402912
##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java:
##########
@@ -729,6 +731,14 @@ public WritableByteChannel create(GcsPath path,
CreateOptions options) throws IO
GoogleCloudStorage createGoogleCloudStorage(
GoogleCloudStorageOptions options, Storage storage, Credentials
credentials)
throws IOException {
+ // Suppress log spams in gcsio 3.0
+ if (overwriteLog.compareAndSet(false, true)) {
+ java.util.logging.Logger gcsLogger =
+ java.util.logging.Logger.getLogger(
+ "com.google.cloud.hadoop.gcsio.GoogleCloudStorageImpl");
+ gcsLogger.setLevel(java.util.logging.Level.SEVERE);
+ }
Review Comment:
Using `GoogleCloudStorageImpl.class.getName()` is preferred over a hardcoded
string to ensure type safety and ease of refactoring. Additionally, please note
that setting the log level to `SEVERE` is very aggressive as it suppresses all
`WARNING` and `INFO` messages from this class, which might contain important
diagnostic information. If the intention is only to suppress the specific
"ALERT" message, consider if `WARNING` is sufficient or if this suppression
should be made configurable.
```java
if (gcsioLoggerConfigured.compareAndSet(false, true)) {
java.util.logging.Logger gcsLogger =
java.util.logging.Logger.getLogger(GoogleCloudStorageImpl.class.getName());
gcsLogger.setLevel(java.util.logging.Level.SEVERE);
}
```
##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/util/GcsUtilV1.java:
##########
@@ -184,6 +185,7 @@ public boolean shouldRetry(IOException e) {
return RetryDeterminer.SOCKET_ERRORS.shouldRetry(e);
}
};
+ private static final AtomicBoolean overwriteLog = new AtomicBoolean(false);
Review Comment:
The variable name `overwriteLog` is somewhat ambiguous and could be
interpreted as overwriting a log file. Renaming it to something like
`gcsioLoggerConfigured` would more clearly indicate that it tracks whether the
logging configuration for the GCS IO library has been applied.
```suggestion
private static final AtomicBoolean gcsioLoggerConfigured = new
AtomicBoolean(false);
```
--
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]
