You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/06/26 17:06:02 UTC
[GitHub] [dolphinscheduler] WangJPLeo opened a new pull request, #10624: [Feature] Time function analysis extension.
WangJPLeo opened a new pull request, #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
Time function analysis extension.
## Brief change log
1. Add method extension center ExternalFunctionExtensionCenter.
2. Add the default implementation of time function parsing.
3. Add related unit tests.
close #10623
## Verify this pull request
Added UtilsTest to verify the change.
Manually verified the change by testing locally.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1168143754
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10624)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [7 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list) [79.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1166594660
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10624)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list) [88.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] WangJPLeo commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
WangJPLeo commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r907241839
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/ExternalFunctionExtensionCenter.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.expand;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class ExternalFunctionExtensionCenter {
+
+ @Autowired
+ private TimePlaceholderResolverExpandService expandService;
+
+ private static ExternalFunctionExtensionCenter expandCenter;
+
+ @PostConstruct
+ public void init() {
+ expandCenter = this;
+ expandCenter.expandService = this.expandService;
+ }
+
+ public static boolean timeFunctionNeedExpand(String placeholderName) {
+ return expandCenter.expandService.timeFunctionNeedExpand(placeholderName);
+ }
+
+ public static String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ return expandCenter.expandService.timeFunctionExtension(processInstanceId, timeZone, placeholderName);
+ }
+}
Review Comment:
yes, this is indeed a matter of principle. At first, I thought that although the method call is inappropriate, it is thread-safe. I will make a suitable extension optimization for this, thank you for your review.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r907196504
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/ExternalFunctionExtensionCenter.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.expand;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class ExternalFunctionExtensionCenter {
+
+ @Autowired
+ private TimePlaceholderResolverExpandService expandService;
+
+ private static ExternalFunctionExtensionCenter expandCenter;
+
+ @PostConstruct
+ public void init() {
+ expandCenter = this;
+ expandCenter.expandService = this.expandService;
+ }
+
+ public static boolean timeFunctionNeedExpand(String placeholderName) {
+ return expandCenter.expandService.timeFunctionNeedExpand(placeholderName);
+ }
+
+ public static String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ return expandCenter.expandService.timeFunctionExtension(processInstanceId, timeZone, placeholderName);
+ }
+}
Review Comment:
And if someone use this method in not spring object, he will get error.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] WangJPLeo commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
WangJPLeo commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1168149284
> LGTM. It's better to add example on the interface method, you can do this in other PR.
good tip. ok @ruanwenjun
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r907014504
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/TimePlaceholderResolverExpandServiceImpl.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.common.expand;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.stereotype.Component;
+
+@Component
+public class TimePlaceholderResolverExpandServiceImpl implements TimePlaceholderResolverExpandService {
+
+ private static final Logger logger = LoggerFactory.getLogger(TimePlaceholderResolverExpandServiceImpl.class);
+
+ @Override
+ public boolean timeFunctionNeedExpand(String placeholderName) {
+ return false;
+ }
+
+ @Override
+ public String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ logger.warn("time function external expansion");
Review Comment:
remove if not use, and I think the log level is `debug` or `info`, not `warn`.
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java:
##########
@@ -86,7 +89,7 @@ public static String convertParameterPlaceholders(String parameterString, Map<St
* @param scheduleTime schedule time
* @return curing user define parameters
*/
- public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+ public static String curingGlobalParams(Integer processInstanceId, Map<String, String> globalParamMap, List<Property> globalParamList,
Review Comment:
Please update the notes too.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r907196504
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/ExternalFunctionExtensionCenter.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.expand;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class ExternalFunctionExtensionCenter {
+
+ @Autowired
+ private TimePlaceholderResolverExpandService expandService;
+
+ private static ExternalFunctionExtensionCenter expandCenter;
+
+ @PostConstruct
+ public void init() {
+ expandCenter = this;
+ expandCenter.expandService = this.expandService;
+ }
+
+ public static boolean timeFunctionNeedExpand(String placeholderName) {
+ return expandCenter.expandService.timeFunctionNeedExpand(placeholderName);
+ }
+
+ public static String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ return expandCenter.expandService.timeFunctionExtension(processInstanceId, timeZone, placeholderName);
+ }
+}
Review Comment:
And if someone uses this method in not spring object, he will get error.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] lenboo merged pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
lenboo merged PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1167295911
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10624)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list) [86.1% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1166971808
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10624)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list) [80.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] WangJPLeo commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
WangJPLeo commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r907004738
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/ExternalFunctionExtensionCenter.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.expand;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class ExternalFunctionExtensionCenter {
+
+ @Autowired
+ private TimePlaceholderResolverExpandService expandService;
+
+ private static ExternalFunctionExtensionCenter expandCenter;
+
+ @PostConstruct
+ public void init() {
+ expandCenter = this;
+ expandCenter.expandService = this.expandService;
+ }
+
+ public static boolean timeFunctionNeedExpand(String placeholderName) {
+ return expandCenter.expandService.timeFunctionNeedExpand(placeholderName);
+ }
+
+ public static String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ return expandCenter.expandService.timeFunctionExtension(processInstanceId, timeZone, placeholderName);
+ }
+}
Review Comment:
whether a static method causes thread safety depends on whether a static field is used in the static method. If the static method does not operate a static field, only the instance field is used inside the method, which will not cause security problems.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1166593148
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10624?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10624](https://codecov.io/gh/apache/dolphinscheduler/pull/10624?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5569c3f) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/719a9d4532733c0d7b4a54ed52b15dca7982ca8d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (719a9d4) will **increase** coverage by `0.00%`.
> The diff coverage is `86.66%`.
```diff
@@ Coverage Diff @@
## dev #10624 +/- ##
=========================================
Coverage 40.94% 40.95%
- Complexity 4876 4882 +6
=========================================
Files 895 897 +2
Lines 36212 36226 +14
Branches 3987 3988 +1
=========================================
+ Hits 14826 14835 +9
- Misses 19921 19925 +4
- Partials 1465 1466 +1
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/10624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../dolphinscheduler/common/utils/ParameterUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL1BhcmFtZXRlclV0aWxzLmphdmE=) | `71.42% <50.00%> (-1.71%)` | :arrow_down: |
| [...common/expand/ExternalFunctionExtensionCenter.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2V4cGFuZC9FeHRlcm5hbEZ1bmN0aW9uRXh0ZW5zaW9uQ2VudGVyLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...pand/TimePlaceholderResolverExpandServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL2V4cGFuZC9UaW1lUGxhY2Vob2xkZXJSZXNvbHZlckV4cGFuZFNlcnZpY2VJbXBsLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
| [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2Z1dHVyZS9SZXNwb25zZUZ1dHVyZS5qYXZh) | `81.96% <0.00%> (-1.64%)` | :arrow_down: |
| [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Fvb3Avc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3Fvb3AvcGFyYW1ldGVyL1Nxb29wUGFyYW1ldGVycy5qYXZh) | `53.33% <0.00%> (-1.34%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10624?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10624?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [719a9d4...5569c3f](https://codecov.io/gh/apache/dolphinscheduler/pull/10624?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r907193204
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/ExternalFunctionExtensionCenter.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.expand;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class ExternalFunctionExtensionCenter {
+
+ @Autowired
+ private TimePlaceholderResolverExpandService expandService;
+
+ private static ExternalFunctionExtensionCenter expandCenter;
+
+ @PostConstruct
+ public void init() {
+ expandCenter = this;
+ expandCenter.expandService = this.expandService;
+ }
+
+ public static boolean timeFunctionNeedExpand(String placeholderName) {
+ return expandCenter.expandService.timeFunctionNeedExpand(placeholderName);
+ }
+
+ public static String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ return expandCenter.expandService.timeFunctionExtension(processInstanceId, timeZone, placeholderName);
+ }
+}
Review Comment:
When someone execute `ExternalFunctionExtensionCenter.timeFunctionNeedExpand`, he will get unknow result, since he cannot sure if this class has been initialized. This is not a good practice, we need to avoid operating a non-static variable in a static method.
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/ExternalFunctionExtensionCenter.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.expand;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class ExternalFunctionExtensionCenter {
+
+ @Autowired
+ private TimePlaceholderResolverExpandService expandService;
+
+ private static ExternalFunctionExtensionCenter expandCenter;
+
+ @PostConstruct
+ public void init() {
+ expandCenter = this;
+ expandCenter.expandService = this.expandService;
+ }
+
+ public static boolean timeFunctionNeedExpand(String placeholderName) {
+ return expandCenter.expandService.timeFunctionNeedExpand(placeholderName);
+ }
+
+ public static String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ return expandCenter.expandService.timeFunctionExtension(processInstanceId, timeZone, placeholderName);
+ }
+}
Review Comment:
When someone execute `ExternalFunctionExtensionCenter.timeFunctionNeedExpand`, he will get unknown result, since he cannot sure if this class has been initialized. This is not a good practice, we need to avoid operating a non-static variable in a static method.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1166600957
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10624)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list) [81.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] WangJPLeo commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
WangJPLeo commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r907030358
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/TimePlaceholderResolverExpandServiceImpl.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.common.expand;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.stereotype.Component;
+
+@Component
+public class TimePlaceholderResolverExpandServiceImpl implements TimePlaceholderResolverExpandService {
+
+ private static final Logger logger = LoggerFactory.getLogger(TimePlaceholderResolverExpandServiceImpl.class);
+
+ @Override
+ public boolean timeFunctionNeedExpand(String placeholderName) {
+ return false;
+ }
+
+ @Override
+ public String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ logger.warn("time function external expansion");
Review Comment:
ok thank you.
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java:
##########
@@ -86,7 +89,7 @@ public static String convertParameterPlaceholders(String parameterString, Map<St
* @param scheduleTime schedule time
* @return curing user define parameters
*/
- public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+ public static String curingGlobalParams(Integer processInstanceId, Map<String, String> globalParamMap, List<Property> globalParamList,
Review Comment:
ok.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1167109091
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10624)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10624&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list) [82.6% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10624&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#discussion_r906911444
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/expand/ExternalFunctionExtensionCenter.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common.expand;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+@Component
+public class ExternalFunctionExtensionCenter {
+
+ @Autowired
+ private TimePlaceholderResolverExpandService expandService;
+
+ private static ExternalFunctionExtensionCenter expandCenter;
+
+ @PostConstruct
+ public void init() {
+ expandCenter = this;
+ expandCenter.expandService = this.expandService;
+ }
+
+ public static boolean timeFunctionNeedExpand(String placeholderName) {
+ return expandCenter.expandService.timeFunctionNeedExpand(placeholderName);
+ }
+
+ public static String timeFunctionExtension(Integer processInstanceId, String timeZone, String placeholderName) {
+ return expandCenter.expandService.timeFunctionExtension(processInstanceId, timeZone, placeholderName);
+ }
+}
Review Comment:
Please remove `static`, this is not safe.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] WangJPLeo commented on pull request #10624: [Feature] Time function analysis extension.
Posted by GitBox <gi...@apache.org>.
WangJPLeo commented on PR #10624:
URL: https://github.com/apache/dolphinscheduler/pull/10624#issuecomment-1168149049
> LGTM. It's better to add example on the interface method, you can do this in other PR.
good tip. ok
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org