caishunfeng commented on code in PR #16413:
URL:
https://github.com/apache/dolphinscheduler/pull/16413#discussion_r1702977810
##########
dolphinscheduler-ui/src/views/projects/task/components/node/fields/use-http.ts:
##########
@@ -128,14 +165,30 @@ export function useHttp(model: { [field: string]: any }):
IJsonItem[] {
]
},
{
- type: 'input',
+ type: 'editor',
field: 'httpBody',
name: t('project.node.http_body'),
- props: {
- type: 'textarea',
- placeholder: t('project.node.http_body_tips')
+ span: httpBodySpan,
+ validate: {
+ trigger: ['blur', 'input'],
+ required: true,
+ validator(validate, value) {
+ if (httpBodySpan.value && !value) {
+ return new Error(t('project.node.http_body_tips'))
+ }
+ }
}
},
+ // {
Review Comment:
delete if not use.
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/OkHttpUtils.java:
##########
@@ -35,53 +39,116 @@
public class OkHttpUtils {
- private static final OkHttpClient CLIENT = new OkHttpClient.Builder()
- .connectTimeout(5, TimeUnit.MINUTES) // connect timeout
- .writeTimeout(5, TimeUnit.MINUTES) // write timeout
- .readTimeout(5, TimeUnit.MINUTES)
- .build();
-
- public static @NonNull String get(@NonNull String url,
- @Nullable Map<String, String>
httpHeaders,
- @Nullable Map<String, Object>
requestParams) throws IOException {
+ private static OkHttpClient CLIENT = new OkHttpClient();
+
+ public static @NonNull OkHttpResponse get(@NonNull String url,
Review Comment:
Please add some method description, especially some units of time.
Same for other methods.
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/LoginController.java:
##########
@@ -223,16 +225,28 @@ public void loginByAuth2(@RequestParam String code,
@RequestParam String provide
requestParamsMap.put("grant_type", "authorization_code");
requestParamsMap.put("redirect_uri",
String.format("%s?provider=%s",
oAuth2ClientProperties.getRedirectUri(), provider));
- String tokenJsonStr =
OkHttpUtils.post(oAuth2ClientProperties.getTokenUri(), tokenRequestHeader,
- requestParamsMap, requestBody);
+ OkHttpRequestHeaders okHttpRequestHeadersPost = new
OkHttpRequestHeaders();
+ okHttpRequestHeadersPost.setHeaders(tokenRequestHeader);
+
okHttpRequestHeadersPost.setOkHttpRequestHeaderContentType(OkHttpRequestHeaderContentType.APPLICATION_JSON);
+ int timeout = 30000;
Review Comment:
It's better to use constant but not magic number.
##########
tools/dependencies/known-dependencies.txt:
##########
@@ -251,8 +251,9 @@ netty-transport-native-unix-common-4.1.53.Final.jar
nimbus-jose-jwt-9.8.1.jar
nimbus-jose-jwt-9.10.jar
okhttp-2.7.5.jar
-okhttp-4.9.3.jar
-okio-2.8.0.jar
+okhttp-4.12.0.jar
+okio-3.6.0.jar
+okio-jvm-3.6.0.jar
Review Comment:
Should update the license?
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/model/OkHttpRequestHeaderContentType.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to 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. Apache Software Foundation (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.common.model;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+@Getter
+@RequiredArgsConstructor
+public enum OkHttpRequestHeaderContentType {
+
+ APPLICATION_JSON("application/json"),
+ APPLICATION_FORM_URLENCODED("application/x-www-form-urlencoded");
Review Comment:
Why not use `org.apache.http.entity.ContentType`, which contains more
complete enum.
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-http/src/main/java/org/apache/dolphinscheduler/plugin/task/http/HttpParameters.java:
##########
@@ -17,47 +17,37 @@
package org.apache.dolphinscheduler.plugin.task.http;
-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.ArrayList;
import java.util.List;
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
/**
* http parameter
*/
+@EqualsAndHashCode(callSuper = true)
+@Data
public class HttpParameters extends AbstractParameters {
- /**
- * url
- */
private String url;
- /**
- * httpMethod
- */
- private HttpMethod httpMethod;
+ @JsonProperty("httpMethod")
+ private HttpRequestMethod httpRequestMethod;
- /**
- * http params
- */
- private List<HttpProperty> httpParams;
+ @JsonProperty("httpParams")
+ private List<HttpProperty> httpRequestParams;
Review Comment:
It seems not necessary to change these field names.
--
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]