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 2019/12/27 10:23:01 UTC

[GitHub] [incubator-dolphinscheduler] samz406 opened a new pull request #1618: before delete Udf check exist

samz406 opened a new pull request #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618
 
 
   
   ## What is the purpose of the pull request
   
   1 Modify judgment order(调整顺序，先判断是否存在)
   2 before delete check entity exist

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] Technoboy- commented on a change in pull request #1618: before delete Udf check exist

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618#discussion_r361790999
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
 ##########
 @@ -303,7 +303,12 @@ private boolean checkUdfFuncNameExists(String name){
     @Transactional(rollbackFor = Exception.class)
     public Result delete(int id) {
         Result result = new Result();
-
+        //check exist
 
 Review comment:
   What is the benefit of the checking logic before deleting ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] codecov-io edited a comment on issue #1618: before delete Udf check exist

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618#issuecomment-569241860
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=h1) Report
   > Merging [#1618](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=desc) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/be5fc116effe456cb9b74b277a3fb5c3988e6870?src=pr&el=desc) will **increase** coverage by `5.29%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/graphs/tree.svg?width=650&token=bv9iXXRLi9&height=150&src=pr)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff            @@
   ##             dev   #1618      +/-   ##
   ========================================
   + Coverage   8.11%   13.4%   +5.29%     
   ========================================
     Files        282     282              
     Lines      13785   13915     +130     
     Branches    2201    2250      +49     
   ========================================
   + Hits        1118    1866     +748     
   + Misses     12583   11843     -740     
   - Partials      84     206     +122
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...e/dolphinscheduler/api/service/UdfFuncService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvVWRmRnVuY1NlcnZpY2UuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...inscheduler/alert/utils/EnterpriseWeChatUtils.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9hbGVydC91dGlscy9FbnRlcnByaXNlV2VDaGF0VXRpbHMuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...e/dolphinscheduler/common/zk/AbstractZKClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3prL0Fic3RyYWN0WktDbGllbnQuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...org/apache/dolphinscheduler/alert/AlertServer.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9hbGVydC9BbGVydFNlcnZlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...e/dolphinscheduler/server/master/MasterServer.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9NYXN0ZXJTZXJ2ZXIuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...e/dolphinscheduler/server/worker/WorkerServer.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9Xb3JrZXJTZXJ2ZXIuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...e/dolphinscheduler/common/utils/PropertyUtils.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL1Byb3BlcnR5VXRpbHMuamF2YQ==) | `32.14% <0%> (ø)` | :arrow_up: |
   | [.../org/apache/dolphinscheduler/common/Constants.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL0NvbnN0YW50cy5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...che/dolphinscheduler/server/zk/ZKMasterClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3prL1pLTWFzdGVyQ2xpZW50LmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
   | [...olphinscheduler/api/controller/BaseController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQmFzZUNvbnRyb2xsZXIuamF2YQ==) | `3.65% <0%> (+3.65%)` | :arrow_up: |
   | ... and [16 more](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=footer). Last update [be5fc11...96d49a0](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] codecov-io commented on issue #1618: before delete Udf check exist

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618#issuecomment-569241860
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=h1) Report
   > Merging [#1618](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=desc) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/be5fc116effe456cb9b74b277a3fb5c3988e6870?src=pr&el=desc) will **decrease** coverage by `<.01%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/graphs/tree.svg?width=650&token=bv9iXXRLi9&height=150&src=pr)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff            @@
   ##             dev   #1618      +/-   ##
   ========================================
   - Coverage   8.11%    8.1%   -0.01%     
   ========================================
     Files        282     282              
     Lines      13785   13789       +4     
     Branches    2201    2202       +1     
   ========================================
     Hits        1118    1118              
   - Misses     12583   12587       +4     
     Partials      84      84
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...e/dolphinscheduler/api/service/UdfFuncService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvVWRmRnVuY1NlcnZpY2UuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=footer). Last update [be5fc11...62f54ee](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/1618?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] samz406 commented on a change in pull request #1618: before delete Udf check exist

Posted by GitBox <gi...@apache.org>.
samz406 commented on a change in pull request #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618#discussion_r361793368
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
 ##########
 @@ -303,7 +303,12 @@ private boolean checkUdfFuncNameExists(String name){
     @Transactional(rollbackFor = Exception.class)
     public Result delete(int id) {
         Result result = new Result();
-
+        //check exist
 
 Review comment:
   I have expressed a problem, this is not to say that the delete order is adjusted. The delete method modifies the question. Order adjustment refers to the checkUdfFuncNameExists method. My understanding is that you should first determine if udf exists and then check if hdfs is started.
   我表述的有问题，这里不是说delete顺序调整。delete方法修改的是提示问题。顺序调整是指checkUdfFuncNameExists方法。我的理解是应该先判断udf是否存在然后再去检查hdfs是否启动。

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] Technoboy- commented on a change in pull request #1618: before delete Udf check exist

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618#discussion_r361795630
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
 ##########
 @@ -303,7 +303,12 @@ private boolean checkUdfFuncNameExists(String name){
     @Transactional(rollbackFor = Exception.class)
     public Result delete(int id) {
         Result result = new Result();
-
+        //check exist
 
 Review comment:
   For delete method only, in line 304.
   What is the benefit of the checking logic before deleting ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] Technoboy- merged pull request #1618: before delete Udf check exist

Posted by GitBox <gi...@apache.org>.
Technoboy- merged pull request #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-dolphinscheduler] Technoboy- commented on a change in pull request #1618: before delete Udf check exist

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1618: before delete Udf check exist
URL: https://github.com/apache/incubator-dolphinscheduler/pull/1618#discussion_r361795646
 
 

 ##########
 File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
 ##########
 @@ -303,7 +303,12 @@ private boolean checkUdfFuncNameExists(String name){
     @Transactional(rollbackFor = Exception.class)
     public Result delete(int id) {
         Result result = new Result();
-
+        //check exist
 
 Review comment:
   all the checking logic is nice above line 304 .

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services