zixi0825 commented on code in PR #16863:
URL: 
https://github.com/apache/dolphinscheduler/pull/16863#discussion_r3791730275


##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/java/org/apache/dolphinscheduler/e2e/cases/tasks/DatavinesTaskE2ETest.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.e2e.cases.tasks;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.dolphinscheduler.e2e.cases.workflow.BaseWorkflowE2ETest;
+import org.apache.dolphinscheduler.e2e.core.DolphinScheduler;
+import org.apache.dolphinscheduler.e2e.core.WebDriverHolder;
+import org.apache.dolphinscheduler.e2e.pages.LoginPage;
+import org.apache.dolphinscheduler.e2e.pages.project.ProjectPage;
+import org.apache.dolphinscheduler.e2e.pages.project.workflow.TaskInstanceTab;
+import 
org.apache.dolphinscheduler.e2e.pages.project.workflow.WorkflowDefinitionTab;
+import org.apache.dolphinscheduler.e2e.pages.project.workflow.WorkflowForm;
+import 
org.apache.dolphinscheduler.e2e.pages.project.workflow.WorkflowInstanceTab;
+import 
org.apache.dolphinscheduler.e2e.pages.project.workflow.task.DatavinesTaskForm;
+import org.apache.dolphinscheduler.e2e.pages.security.SecurityPage;
+import org.apache.dolphinscheduler.e2e.pages.security.TenantPage;
+import org.apache.dolphinscheduler.e2e.pages.security.UserPage;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junitpioneer.jupiter.DisableIfTestFails;
+
+@TestMethodOrder(MethodOrderer.MethodName.class)
+@DolphinScheduler(composeFiles = "docker/datavines-task/docker-compose.yaml")
+@DisableIfTestFails
+public class DatavinesTaskE2ETest extends BaseWorkflowE2ETest {

Review Comment:
   Thanks for the review. `DatavinesTaskE2ETest` has been added to the E2E 
matrix in `.github/workflows/e2e.yml` (commit 9b02337514).



##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/java/org/apache/dolphinscheduler/e2e/cases/tasks/DatavinesTaskE2ETest.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.e2e.cases.tasks;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.dolphinscheduler.e2e.cases.workflow.BaseWorkflowE2ETest;
+import org.apache.dolphinscheduler.e2e.core.DolphinScheduler;
+import org.apache.dolphinscheduler.e2e.core.WebDriverHolder;
+import org.apache.dolphinscheduler.e2e.pages.LoginPage;
+import org.apache.dolphinscheduler.e2e.pages.project.ProjectPage;
+import org.apache.dolphinscheduler.e2e.pages.project.workflow.TaskInstanceTab;
+import 
org.apache.dolphinscheduler.e2e.pages.project.workflow.WorkflowDefinitionTab;
+import org.apache.dolphinscheduler.e2e.pages.project.workflow.WorkflowForm;
+import 
org.apache.dolphinscheduler.e2e.pages.project.workflow.WorkflowInstanceTab;
+import 
org.apache.dolphinscheduler.e2e.pages.project.workflow.task.DatavinesTaskForm;
+import org.apache.dolphinscheduler.e2e.pages.security.SecurityPage;
+import org.apache.dolphinscheduler.e2e.pages.security.TenantPage;
+import org.apache.dolphinscheduler.e2e.pages.security.UserPage;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junitpioneer.jupiter.DisableIfTestFails;
+
+@TestMethodOrder(MethodOrderer.MethodName.class)
+@DolphinScheduler(composeFiles = "docker/datavines-task/docker-compose.yaml")
+@DisableIfTestFails
+public class DatavinesTaskE2ETest extends BaseWorkflowE2ETest {
+
+    private static final String mockServerAddress = "http://mockServer:1080";;
+
+    private static final String jobId = "1";
+
+    private static final String token = "test-token";
+
+    @BeforeAll
+    public static void setup() {
+        browser = WebDriverHolder.getWebDriver();
+
+        TenantPage tenantPage = new LoginPage(browser)
+                .login(adminUser)
+                .goToNav(SecurityPage.class)
+                .goToTab(TenantPage.class);
+
+        if (tenantPage.tenants().stream().noneMatch(tenant -> 
tenant.tenantCode().equals(adminUser.getTenant()))) {
+            tenantPage
+                    .create(adminUser.getTenant())
+                    .goToNav(SecurityPage.class)
+                    .goToTab(UserPage.class)
+                    .update(adminUser);
+        }
+
+        tenantPage
+                .goToNav(ProjectPage.class)
+                .createProjectUntilSuccess(projectName);
+    }
+
+    @Test
+    @Order(10)
+    void testRunDatavinesTasks_SuccessCase() {

Review Comment:
   Thanks. A failure case has been added: `testRunDatavinesTasks_FailureCase` 
covers the execution-failure path (jobId 2), and 
`testRunDatavinesTasks_FailureBlockDisabledCase` covers the quality-check 
failure with failureBlock disabled (commits 9b02337514 and 4cb4274b0e).



##########
dolphinscheduler-e2e/dolphinscheduler-e2e-case/src/test/resources/docker/datavines-task/docker-compose.yaml:
##########
@@ -0,0 +1,55 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+version: "3.8"
+
+services:
+  dolphinscheduler:
+    image: apache/dolphinscheduler-standalone-server:ci
+    environment:
+      MASTER_MAX_CPU_LOAD_AVG: 400
+      MASTER_RESERVED_MEMORY: 0.01
+      WORKER_TENANT_AUTO_CREATE: 'true'

Review Comment:
   Thanks. The docker-compose configuration has been aligned with the other E2E 
cases (same standalone server environment and healthcheck as 
`docker/basic/docker-compose.yaml`, plus a mockServer service for the Datavines 
API) (commit 9b02337514).



##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datavines/src/main/java/org/apache/dolphinscheduler/plugin/task/datavines/utils/RequestUtils.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.plugin.task.datavines.utils;
+
+import 
org.apache.dolphinscheduler.plugin.task.datavines.DatavinesTaskConstants;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.utils.URIBuilder;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.util.EntityUtils;
+
+import java.net.URI;
+
+import lombok.extern.slf4j.Slf4j;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+@Slf4j
+public class RequestUtils {
+
+    private RequestUtils() {
+        throw new IllegalStateException("Utility class");
+    }
+
+    public static JsonNode executeJob(String address, String jobId, String 
token) {
+        return parse(doPost(address + DatavinesTaskConstants.EXECUTE_JOB + 
jobId, token));
+    }
+
+    public static JsonNode getJobExecutionStatus(String address, String 
jobExecutionId, String token) {
+        return parse(doGet(address + 
DatavinesTaskConstants.GET_JOB_EXECUTION_STATUS + jobExecutionId, token));
+    }
+
+    public static JsonNode getJobExecutionResult(String address, String 
jobExecutionId, String token) {
+        return parse(doGet(address + 
DatavinesTaskConstants.GET_JOB_EXECUTION_RESULT + jobExecutionId, token));
+    }
+
+    public static void killJobExecution(String address, String jobExecutionId, 
String token) {
+        parse(doPost(address + DatavinesTaskConstants.JOB_EXECUTION_KILL + 
jobExecutionId, token));
+    }
+
+    public static JsonNode parse(String res) {

Review Comment:
   Thanks. `RequestUtils.parse` now reuses `JSONUtils.parseObject` instead of 
creating a new ObjectMapper (commit 9b02337514).



##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datavines/src/main/java/org/apache/dolphinscheduler/plugin/task/datavines/DatavinesParameters.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.plugin.task.datavines;
+
+import org.apache.dolphinscheduler.plugin.task.api.model.ResourceInfo;
+import 
org.apache.dolphinscheduler.plugin.task.api.parameters.AbstractParameters;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Collections;
+import java.util.List;
+
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+
+@EqualsAndHashCode(callSuper = true)
+@Data
+public class DatavinesParameters extends AbstractParameters {
+
+    private String address;
+
+    private String jobId;
+
+    private String token;
+
+    private boolean failureBlock;
+
+    @Override
+    public boolean checkParameters() {
+        return StringUtils.isNotEmpty(this.address) && 
StringUtils.isNotEmpty(this.jobId);

Review Comment:
   Thanks. `checkParameters()` now also validates that `token` is not empty 
(commit 9b02337514).



-- 
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]

Reply via email to