junkaixue commented on code in PR #2987:
URL: https://github.com/apache/helix/pull/2987#discussion_r1917250681


##########
helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java:
##########
@@ -111,13 +115,29 @@ public Builder setReason(String reason) {
 
       /**
        * Set the source for this instance operation.
-       * @param source InstanceOperationSource
-       *              that caused this instance operation to be triggered.
+       *
+       * @param source InstanceOperationSource that caused this instance 
operation to be triggered.
        */
       public Builder setSource(InstanceConstants.InstanceOperationSource 
source) {
         _properties.put(InstanceOperationProperties.SOURCE.name(),
-            source == null ? 
InstanceConstants.InstanceOperationSource.USER.name()
-                : source.name());
+            source == null ? 
InstanceConstants.InstanceOperationSource.USER.name() : source.name());
+        return this;
+      }
+
+      /**
+       * Set the HELIX_DISABLED_TYPE which is a legacy field that must be 
stored, so we can write it
+       * back when we write back to the legacy fields.
+       *
+       * @param disabledType InstanceDisabledType
+       * @return
+       * @throws IllegalArgumentException
+       */
+      private Builder 
setLegacyDisabledType(InstanceConstants.InstanceDisabledType disabledType) {
+        if (disabledType == null) {
+          return this;

Review Comment:
   log it or throw exception?



##########
helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java:
##########
@@ -67,14 +67,16 @@ public enum InstanceConfigProperty {
     DELAY_REBALANCE_ENABLED,
     MAX_CONCURRENT_TASK,
     INSTANCE_INFO_MAP,
-    INSTANCE_CAPACITY_MAP, TARGET_TASK_THREAD_POOL_SIZE, 
HELIX_INSTANCE_OPERATIONS
+    INSTANCE_CAPACITY_MAP,
+    TARGET_TASK_THREAD_POOL_SIZE,
+    HELIX_INSTANCE_OPERATIONS
   }
 
   public static class InstanceOperation {
     private final Map<String, String> _properties;
 
     private enum InstanceOperationProperties {
-      OPERATION, REASON, SOURCE, TIMESTAMP
+      OPERATION, REASON, SOURCE, TIMESTAMP, LEGACY_DISABLED_TYPE

Review Comment:
   Be consistent with formatting? I think I fixed Helix formatting in 
Helix-Style for intellij for this case. Please try it out.
   
   If not, maybe fix 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: reviews-unsubscr...@helix.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org

Reply via email to