ruanwenjun commented on code in PR #13399:
URL:
https://github.com/apache/dolphinscheduler/pull/13399#discussion_r1070623449
##########
dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/log/LogClientTest.java:
##########
@@ -151,8 +151,7 @@ public void testRemoveTaskLog() throws Exception {
.thenReturn(command);
LogClient logClient = new LogClient();
- Boolean status =
logClient.removeTaskLog(Host.of("localhost:1234"), "/log/path");
- Assertions.assertTrue(status);
+ Assertions.assertDoesNotThrow(() ->
logClient.removeTaskLog(Host.of("localhost:1234"), "/log/path"));
Review Comment:
Yes, this assertion is only want to make sure this method will never throw
exception.
##########
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/log/LogClient.java:
##########
@@ -179,27 +178,14 @@ public byte[] getLogBytes(String host, int port, String
path) {
* @param path path
* @return remove task status
*/
- public Boolean removeTaskLog(@NonNull Host host, String path) {
Review Comment:
Done
##########
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/log/LogClient.java:
##########
@@ -179,27 +178,14 @@ public byte[] getLogBytes(String host, int port, String
path) {
* @param path path
* @return remove task status
*/
- public Boolean removeTaskLog(@NonNull Host host, String path) {
+ public void removeTaskLog(@NonNull Host host, String path) {
logger.info("Remove task log from host: {} logPath {}", host, path);
RemoveTaskLogRequestCommand request = new
RemoveTaskLogRequestCommand(path);
try {
Command command = request.convert2Command();
- Command response = this.client.sendSync(host, command,
LOG_REQUEST_TIMEOUT);
- if (response != null) {
- RemoveTaskLogResponseCommand taskLogResponse =
- JSONUtils.parseObject(response.getBody(),
RemoveTaskLogResponseCommand.class);
- return taskLogResponse.getStatus();
- }
- return false;
- } catch (InterruptedException ex) {
- Thread.currentThread().interrupt();
- logger.error(
- "Remove task log from host: {}, logPath: {} error, the
current thread has been interrupted",
- host, path, ex);
- return false;
+ client.send(host, command);
Review Comment:
Good succession, I change this to `sendAsync` and add callback to log the
result.
--
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]
