You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/11/14 04:14:54 UTC

[GitHub] [inlong] healchow opened a new pull request, #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

healchow opened a new pull request, #6524:
URL: https://github.com/apache/inlong/pull/6524

   ### Prepare a Pull Request
   
   - Fixes #6517
   
   ### Motivation
   
   Support delete stream sink when the inlong group was config_failed.
   
   ### Modifications
   
   
   ### Verifying this change
   
   - [x] This change is a trivial rework/code cleanup without any test coverage.
   


-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] healchow commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021129804


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/group/InlongGroupServiceImpl.java:
##########
@@ -435,7 +435,7 @@ public InlongGroupInfo doDeleteCheck(String groupId, String operator) {
 
         // If the status allowed logic delete, all associated info will be logically deleted.
         // In other status, you need to delete the related "inlong_stream" first.
-        if (!GroupStatus.allowedLogicDelete(curState)) {
+        if (!GroupStatus.notAllowedDelete(curState)) {
             int count = streamService.selectCountByGroupId(groupId);

Review Comment:
   When the inlong group supports the delete operation, we should check whether there are undeleted streams.



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] healchow commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021383013


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/group/InlongGroupServiceImpl.java:
##########
@@ -465,7 +465,7 @@ public Boolean delete(String groupId, String operator) {
         // logically delete the associated extension info
         groupExtMapper.logicDeleteAllByGroupId(groupId);
 
-        if (GroupStatus.allowedLogicDelete(GroupStatus.forCode(entity.getStatus()))) {
+        if (GroupStatus.notAllowedDelete(GroupStatus.forCode(entity.getStatus()))) {
             streamService.logicDeleteAll(groupId, operator);

Review Comment:
   Fixed.



##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/GroupStatus.java:
##########
@@ -117,40 +115,55 @@ public static boolean notAllowedUpdate(GroupStatus status) {
     }
 
     /**
-     * Checks whether the given status allows deleting operate.
+     * Checks whether the given status allows updating the MQ type of inlong group.
      */
-    public static boolean notAllowedDelete(GroupStatus status) {
-        return status == GroupStatus.TO_BE_APPROVAL
-                || status == GroupStatus.CONFIG_ING
-                || status == GroupStatus.SUSPENDING
-                || status == GroupStatus.RESTARTING;
+    public static boolean allowedUpdateMQ(GroupStatus status) {
+        return status == GroupStatus.TO_BE_SUBMIT
+                || status == GroupStatus.TO_BE_APPROVAL
+                || status == GroupStatus.APPROVE_REJECTED
+                || status == GroupStatus.CONFIG_FAILED;
     }
 
     /**
-     * Checks whether the given status allows suspending operate.
+     * Checks whether the given status needs to delete the inlong stream first.
      */
-    public static boolean allowedSuspend(GroupStatus status) {
-        return status == GroupStatus.CONFIG_SUCCESSFUL
-                || status == GroupStatus.RESTARTED
+    public static boolean deleteStreamFirst(GroupStatus status) {
+        return status == GroupStatus.APPROVE_PASSED
+                || status == GroupStatus.CONFIG_FAILED
+                || status == GroupStatus.CONFIG_SUCCESSFUL
                 || status == GroupStatus.SUSPENDED
+                || status == GroupStatus.RESTARTED
                 || status == GroupStatus.FINISH;
     }
 
     /**
-     * Only the {@link GroupStatus#DRAFT} and {@link GroupStatus#TO_BE_SUBMIT} status
-     * allows change the MQ type of inlong group.
+     * Checks whether the given status allows deleting other infos,
+     * <p/>
+     * If true, will logically delete all related infos, including streams, sources, sinks, etc.
+     */
+    public static boolean allowedDeleteSubInfos(GroupStatus status) {
+        return status == GroupStatus.TO_BE_SUBMIT
+                || status == GroupStatus.APPROVE_REJECTED
+                || status == GroupStatus.DELETED;
+    }
+
+    /**
+     * Checks whether the given status allows suspending operate.
      */
-    public static boolean notAllowedUpdateMQ(GroupStatus status) {
-        return status != GroupStatus.DRAFT && status != GroupStatus.TO_BE_SUBMIT
-                && status != GroupStatus.TO_BE_APPROVAL && status != GroupStatus.APPROVE_REJECTED
-                && status != GroupStatus.CONFIG_FAILED;
+    public static boolean allowedSuspend(GroupStatus status) {
+        return status == GroupStatus.CONFIG_SUCCESSFUL
+                || status == GroupStatus.RESTARTED
+                || status == GroupStatus.SUSPENDED
+                || status == GroupStatus.FINISH;
     }
 
     /**
      * Temporary group status, adding, deleting and modifying operations are not allowed
      */
     public static boolean isTempStatus(GroupStatus status) {
-        return status == TO_BE_APPROVAL || status == CONFIG_ING;
+        return status == TO_BE_APPROVAL || status == CONFIG_ING
+                || status == SUSPENDING || status == RESTARTING
+                ;

Review Comment:
   Done, thanks.



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] gong commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021098059


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/group/InlongGroupServiceImpl.java:
##########
@@ -465,7 +465,7 @@ public Boolean delete(String groupId, String operator) {
         // logically delete the associated extension info
         groupExtMapper.logicDeleteAllByGroupId(groupId);
 
-        if (GroupStatus.allowedLogicDelete(GroupStatus.forCode(entity.getStatus()))) {
+        if (GroupStatus.notAllowedDelete(GroupStatus.forCode(entity.getStatus()))) {
             streamService.logicDeleteAll(groupId, operator);

Review Comment:
   Maybe, `GroupStatus.notAllowedDelete(GroupStatus.forCode(entity.getStatus()))`  modify to `!GroupStatus.notAllowedDelete(GroupStatus.forCode(entity.getStatus()))`



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] healchow commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1022230212


##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/SimpleGroupStatus.java:
##########
@@ -36,7 +36,6 @@ public enum SimpleGroupStatus {
     public static SimpleGroupStatus parseStatusByCode(int code) {
         GroupStatus groupStatus = GroupStatus.forCode(code);
         switch (groupStatus) {
-            case DRAFT:

Review Comment:
   Because this status is not used anywhere.



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] fuweng11 commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
fuweng11 commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021375976


##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/GroupStatus.java:
##########
@@ -117,40 +115,55 @@ public static boolean notAllowedUpdate(GroupStatus status) {
     }
 
     /**
-     * Checks whether the given status allows deleting operate.
+     * Checks whether the given status allows updating the MQ type of inlong group.
      */
-    public static boolean notAllowedDelete(GroupStatus status) {
-        return status == GroupStatus.TO_BE_APPROVAL
-                || status == GroupStatus.CONFIG_ING
-                || status == GroupStatus.SUSPENDING
-                || status == GroupStatus.RESTARTING;
+    public static boolean allowedUpdateMQ(GroupStatus status) {
+        return status == GroupStatus.TO_BE_SUBMIT
+                || status == GroupStatus.TO_BE_APPROVAL
+                || status == GroupStatus.APPROVE_REJECTED
+                || status == GroupStatus.CONFIG_FAILED;
     }
 
     /**
-     * Checks whether the given status allows suspending operate.
+     * Checks whether the given status needs to delete the inlong stream first.
      */
-    public static boolean allowedSuspend(GroupStatus status) {
-        return status == GroupStatus.CONFIG_SUCCESSFUL
-                || status == GroupStatus.RESTARTED
+    public static boolean deleteStreamFirst(GroupStatus status) {
+        return status == GroupStatus.APPROVE_PASSED
+                || status == GroupStatus.CONFIG_FAILED
+                || status == GroupStatus.CONFIG_SUCCESSFUL
                 || status == GroupStatus.SUSPENDED
+                || status == GroupStatus.RESTARTED
                 || status == GroupStatus.FINISH;
     }
 
     /**
-     * Only the {@link GroupStatus#DRAFT} and {@link GroupStatus#TO_BE_SUBMIT} status
-     * allows change the MQ type of inlong group.
+     * Checks whether the given status allows deleting other infos,
+     * <p/>
+     * If true, will logically delete all related infos, including streams, sources, sinks, etc.
+     */
+    public static boolean allowedDeleteSubInfos(GroupStatus status) {
+        return status == GroupStatus.TO_BE_SUBMIT
+                || status == GroupStatus.APPROVE_REJECTED
+                || status == GroupStatus.DELETED;
+    }
+
+    /**
+     * Checks whether the given status allows suspending operate.
      */
-    public static boolean notAllowedUpdateMQ(GroupStatus status) {
-        return status != GroupStatus.DRAFT && status != GroupStatus.TO_BE_SUBMIT
-                && status != GroupStatus.TO_BE_APPROVAL && status != GroupStatus.APPROVE_REJECTED
-                && status != GroupStatus.CONFIG_FAILED;
+    public static boolean allowedSuspend(GroupStatus status) {
+        return status == GroupStatus.CONFIG_SUCCESSFUL
+                || status == GroupStatus.RESTARTED
+                || status == GroupStatus.SUSPENDED
+                || status == GroupStatus.FINISH;
     }
 
     /**
      * Temporary group status, adding, deleting and modifying operations are not allowed
      */
     public static boolean isTempStatus(GroupStatus status) {
-        return status == TO_BE_APPROVAL || status == CONFIG_ING;
+        return status == TO_BE_APPROVAL || status == CONFIG_ING
+                || status == SUSPENDING || status == RESTARTING
+                ;

Review Comment:
   Whether the temporary status needs to be added `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.

To unsubscribe, e-mail: commits-unsubscribe@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] fuweng11 commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
fuweng11 commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021371056


##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/GroupStatus.java:
##########
@@ -117,40 +115,55 @@ public static boolean notAllowedUpdate(GroupStatus status) {
     }
 
     /**
-     * Checks whether the given status allows deleting operate.
+     * Checks whether the given status allows updating the MQ type of inlong group.
      */
-    public static boolean notAllowedDelete(GroupStatus status) {
-        return status == GroupStatus.TO_BE_APPROVAL
-                || status == GroupStatus.CONFIG_ING
-                || status == GroupStatus.SUSPENDING
-                || status == GroupStatus.RESTARTING;
+    public static boolean allowedUpdateMQ(GroupStatus status) {
+        return status == GroupStatus.TO_BE_SUBMIT
+                || status == GroupStatus.TO_BE_APPROVAL
+                || status == GroupStatus.APPROVE_REJECTED
+                || status == GroupStatus.CONFIG_FAILED;
     }
 
     /**
-     * Checks whether the given status allows suspending operate.
+     * Checks whether the given status needs to delete the inlong stream first.
      */
-    public static boolean allowedSuspend(GroupStatus status) {
-        return status == GroupStatus.CONFIG_SUCCESSFUL
-                || status == GroupStatus.RESTARTED
+    public static boolean deleteStreamFirst(GroupStatus status) {
+        return status == GroupStatus.APPROVE_PASSED
+                || status == GroupStatus.CONFIG_FAILED
+                || status == GroupStatus.CONFIG_SUCCESSFUL
                 || status == GroupStatus.SUSPENDED
+                || status == GroupStatus.RESTARTED
                 || status == GroupStatus.FINISH;
     }
 
     /**
-     * Only the {@link GroupStatus#DRAFT} and {@link GroupStatus#TO_BE_SUBMIT} status
-     * allows change the MQ type of inlong group.
+     * Checks whether the given status allows deleting other infos,
+     * <p/>
+     * If true, will logically delete all related infos, including streams, sources, sinks, etc.
+     */
+    public static boolean allowedDeleteSubInfos(GroupStatus status) {
+        return status == GroupStatus.TO_BE_SUBMIT
+                || status == GroupStatus.APPROVE_REJECTED
+                || status == GroupStatus.DELETED;
+    }
+
+    /**
+     * Checks whether the given status allows suspending operate.
      */
-    public static boolean notAllowedUpdateMQ(GroupStatus status) {
-        return status != GroupStatus.DRAFT && status != GroupStatus.TO_BE_SUBMIT
-                && status != GroupStatus.TO_BE_APPROVAL && status != GroupStatus.APPROVE_REJECTED
-                && status != GroupStatus.CONFIG_FAILED;
+    public static boolean allowedSuspend(GroupStatus status) {
+        return status == GroupStatus.CONFIG_SUCCESSFUL
+                || status == GroupStatus.RESTARTED
+                || status == GroupStatus.SUSPENDED
+                || status == GroupStatus.FINISH;
     }
 
     /**
      * Temporary group status, adding, deleting and modifying operations are not allowed
      */
     public static boolean isTempStatus(GroupStatus status) {
-        return status == TO_BE_APPROVAL || status == CONFIG_ING;
+        return status == TO_BE_APPROVAL || status == CONFIG_ING
+                || status == SUSPENDING || status == RESTARTING
+                ;

Review Comment:
   Comma does not need to start on a new line.



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] gong commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021518219


##########
inlong-manager/manager-common/src/main/java/org/apache/inlong/manager/common/enums/SimpleGroupStatus.java:
##########
@@ -36,7 +36,6 @@ public enum SimpleGroupStatus {
     public static SimpleGroupStatus parseStatusByCode(int code) {
         GroupStatus groupStatus = GroupStatus.forCode(code);
         switch (groupStatus) {
-            case DRAFT:

Review Comment:
   why to delete it?



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] gong commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021098972


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/group/InlongGroupServiceImpl.java:
##########
@@ -435,7 +435,7 @@ public InlongGroupInfo doDeleteCheck(String groupId, String operator) {
 
         // If the status allowed logic delete, all associated info will be logically deleted.
         // In other status, you need to delete the related "inlong_stream" first.
-        if (!GroupStatus.allowedLogicDelete(curState)) {
+        if (!GroupStatus.notAllowedDelete(curState)) {
             int count = streamService.selectCountByGroupId(groupId);

Review Comment:
   Maybe, `!GroupStatus.notAllowedDelete(curState)` modify to `GroupStatus.notAllowedDelete(curState)`



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] healchow merged pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
healchow merged PR #6524:
URL: https://github.com/apache/inlong/pull/6524


-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [inlong] healchow commented on a diff in pull request #6524: [INLONG-6517][Manager] Support delete stream sink when the inlong group was config_failed

Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #6524:
URL: https://github.com/apache/inlong/pull/6524#discussion_r1021129804


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/group/InlongGroupServiceImpl.java:
##########
@@ -435,7 +435,7 @@ public InlongGroupInfo doDeleteCheck(String groupId, String operator) {
 
         // If the status allowed logic delete, all associated info will be logically deleted.
         // In other status, you need to delete the related "inlong_stream" first.
-        if (!GroupStatus.allowedLogicDelete(curState)) {
+        if (!GroupStatus.notAllowedDelete(curState)) {
             int count = streamService.selectCountByGroupId(groupId);

Review Comment:
   When the inlong group supports the delete operate, we should check whether there are undeleted streams.



-- 
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@inlong.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org