Radeity commented on code in PR #13649:
URL:
https://github.com/apache/dolphinscheduler/pull/13649#discussion_r1132163969
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/log/remote/RemoteLogHandlerFactory.java:
##########
@@ -21,19 +21,25 @@
import org.apache.dolphinscheduler.common.utils.PropertyUtils;
import lombok.experimental.UtilityClass;
+import lombok.extern.slf4j.Slf4j;
@UtilityClass
+@Slf4j
public class RemoteLogHandlerFactory {
public RemoteLogHandler getRemoteLogHandler() {
if (!RemoteLogUtils.isRemoteLoggingEnable()) {
return null;
}
- if
(!"OSS".equals(PropertyUtils.getUpperCaseString(Constants.REMOTE_LOGGING_TARGET)))
{
- return null;
+
+ String target =
PropertyUtils.getUpperCaseString(Constants.REMOTE_LOGGING_TARGET);
+ if ("OSS".equals(target)) {
+ return OssRemoteLogHandler.getInstance();
+ } else if ("S3".equals(target)) {
+ return S3RemoteLogHandler.getInstance();
}
- OssRemoteLogHandler ossRemoteLogHandler = new OssRemoteLogHandler();
- ossRemoteLogHandler.init();
- return ossRemoteLogHandler;
+
+ log.error("No suitable remote logging target for {}", target);
Review Comment:
Some tiny suggestion, if we write an error log here, do we have to also
write an error log which may be close in meaning when judging `remoteLogHandler
== null`?
```java
if (remoteLogHandler == null) {
log.error("remote log handler is null");
return;
}
```
--
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]
