You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by jx...@apache.org on 2018/10/29 23:21:27 UTC

[1/3] helix git commit: [HELIX-767] TASK: Remove quotaType fields from Workflow and Job Beans

Repository: helix
Updated Branches:
  refs/heads/master 53a6791e7 -> 739adb0d6


[HELIX-767] TASK: Remove quotaType fields from Workflow and Job Beans

For a short while, we used quota type to denote the quota type of Task Framework resources. However, we changed the design so that we are using the workflowType and jobType fields respectively. There were places where the quotaType field was left over in the codebase, and this RB cleans it up.
Changelist:
1. Remove all quotaType fields from Bean classes
2. Add the setting of workflowType in WorkflowBean when read into Workflow.Builder


Project: http://git-wip-us.apache.org/repos/asf/helix/repo
Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/1d32172a
Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/1d32172a
Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/1d32172a

Branch: refs/heads/master
Commit: 1d32172a53224019b29a6098378daf066ee28e80
Parents: 53a6791
Author: Hunter Lee <hu...@linkedin.com>
Authored: Mon Oct 29 15:50:04 2018 -0700
Committer: Hunter Lee <hu...@linkedin.com>
Committed: Mon Oct 29 15:50:08 2018 -0700

----------------------------------------------------------------------
 helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java | 2 +-
 helix-core/src/main/java/org/apache/helix/task/beans/JobBean.java  | 1 -
 .../src/main/java/org/apache/helix/task/beans/WorkflowBean.java    | 1 -
 3 files changed, 1 insertion(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/helix/blob/1d32172a/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java b/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
index 20da614..0f8a48c 100644
--- a/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
+++ b/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
@@ -623,7 +623,7 @@ public class WorkflowConfig extends ResourceConfig {
         b.setScheduleConfig(ScheduleConfig.from(workflowBean.schedule));
       }
       b.setExpiry(workflowBean.expiry);
-
+      b.setWorkFlowType(workflowBean.workflowType);
       return b;
     }
 

http://git-wip-us.apache.org/repos/asf/helix/blob/1d32172a/helix-core/src/main/java/org/apache/helix/task/beans/JobBean.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/task/beans/JobBean.java b/helix-core/src/main/java/org/apache/helix/task/beans/JobBean.java
index e0f1a70..5674d92 100644
--- a/helix-core/src/main/java/org/apache/helix/task/beans/JobBean.java
+++ b/helix-core/src/main/java/org/apache/helix/task/beans/JobBean.java
@@ -51,5 +51,4 @@ public class JobBean {
   public boolean ignoreDependentJobFailure = JobConfig.DEFAULT_IGNORE_DEPENDENT_JOB_FAILURE;
   public int numberOfTasks = JobConfig.DEFAULT_NUMBER_OF_TASKS;
   public boolean rebalanceRunningTask = JobConfig.DEFAULT_REBALANCE_RUNNING_TASK;
-  public String quotaType;
 }

http://git-wip-us.apache.org/repos/asf/helix/blob/1d32172a/helix-core/src/main/java/org/apache/helix/task/beans/WorkflowBean.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/task/beans/WorkflowBean.java b/helix-core/src/main/java/org/apache/helix/task/beans/WorkflowBean.java
index ae01f5d..2a9563e 100644
--- a/helix-core/src/main/java/org/apache/helix/task/beans/WorkflowBean.java
+++ b/helix-core/src/main/java/org/apache/helix/task/beans/WorkflowBean.java
@@ -32,5 +32,4 @@ public class WorkflowBean {
   public ScheduleBean schedule;
   public long expiry = WorkflowConfig.DEFAULT_EXPIRY;
   public String workflowType;
-  public String quotaType; // Syntactic sugar for setting all of workflow's jobs to this quota type
 }


[3/3] helix git commit: [HELIX-769] TASK2.0: Add PropertyKey APIs for new ZNode structure workflow/job paths

Posted by jx...@apache.org.
[HELIX-769] TASK2.0: Add PropertyKey APIs for new ZNode structure workflow/job paths

As part of ZNode restructuring for Task Framework, we need a convenient way to generate paths to read from and write to ZooKeeper. PropertyKey was already being used for this purpose for the most part throughout Helix, so these APIs were added in PropertyKey and PropertyPathBuilder.

Changelist:
1. Add path generation APIs for Task Framework resources in PropertyKey
2. Add a unit test for the new APIs


Project: http://git-wip-us.apache.org/repos/asf/helix/repo
Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/739adb0d
Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/739adb0d
Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/739adb0d

Branch: refs/heads/master
Commit: 739adb0d6ed35d7281e1c4cbddcffda56a223689
Parents: 65bb350
Author: Hunter Lee <hu...@linkedin.com>
Authored: Mon Oct 29 15:55:29 2018 -0700
Committer: Hunter Lee <hu...@linkedin.com>
Committed: Mon Oct 29 15:55:29 2018 -0700

----------------------------------------------------------------------
 .../main/java/org/apache/helix/PropertyKey.java | 86 +++++++++++++++---
 .../org/apache/helix/PropertyPathBuilder.java   | 61 ++++++++-----
 .../java/org/apache/helix/PropertyType.java     | 22 +++--
 .../helix/util/TestGetWorkflowContext.java      | 58 -------------
 .../helix/util/TestPropertyKeyGetPath.java      | 91 ++++++++++++++++++++
 5 files changed, 222 insertions(+), 96 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/helix/blob/739adb0d/helix-core/src/main/java/org/apache/helix/PropertyKey.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/PropertyKey.java b/helix-core/src/main/java/org/apache/helix/PropertyKey.java
index a5302f8..1ba3108 100644
--- a/helix-core/src/main/java/org/apache/helix/PropertyKey.java
+++ b/helix-core/src/main/java/org/apache/helix/PropertyKey.java
@@ -39,6 +39,9 @@ import org.apache.helix.model.PauseSignal;
 import org.apache.helix.model.ResourceConfig;
 import org.apache.helix.model.StateModelDefinition;
 import org.apache.helix.model.StatusUpdate;
+import org.apache.helix.task.JobConfig;
+import org.apache.helix.task.JobContext;
+import org.apache.helix.task.WorkflowConfig;
 import org.apache.helix.task.WorkflowContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -63,7 +66,8 @@ public class PropertyKey {
    * @param typeClazz
    * @param params parameters associated with the key, the first of which is the cluster name
    */
-  public PropertyKey(PropertyType type, Class<? extends HelixProperty> typeClazz, String... params) {
+  public PropertyKey(PropertyType type, Class<? extends HelixProperty> typeClazz,
+      String... params) {
     this(type, null, typeClazz, params);
   }
 
@@ -227,7 +231,7 @@ public class PropertyKey {
     }
 
     /**
-     * Get a property key associated with resource configurations
+     * Get a property key associated with resource configurations.
      * @return {@link PropertyKey}
      */
     public PropertyKey resourceConfigs() {
@@ -236,7 +240,7 @@ public class PropertyKey {
     }
 
     /**
-     * Get a property key associated with a specific resource configuration
+     * Get a property key associated with a specific resource configuration.
      * @param resourceName
      * @return {@link PropertyKey}
      */
@@ -350,7 +354,6 @@ public class PropertyKey {
       return new PropertyKey(ERRORS, Error.class, _clusterName, instanceName, sessionId);
     }
 
-
     /**
      * Get a property key associated with {@link Error} for an instance under a session of
      * specified resource
@@ -543,7 +546,8 @@ public class PropertyKey {
      * @param msgId
      * @return {@link PropertyKey}
      */
-    public PropertyKey taskError(String instanceName, String sessionId, String msgType, String msgId) {
+    public PropertyKey taskError(String instanceName, String sessionId, String msgType,
+        String msgId) {
       return new PropertyKey(ERRORS, null, _clusterName, instanceName, sessionId, msgType, msgId);
     }
 
@@ -690,7 +694,8 @@ public class PropertyKey {
      * @return {@link PropertyKey}
      */
     public PropertyKey healthReport(String instanceName, String id) {
-      return new PropertyKey(PropertyType.HEALTHREPORT, HealthStat.class, _clusterName, instanceName, id);
+      return new PropertyKey(PropertyType.HEALTHREPORT, HealthStat.class, _clusterName,
+          instanceName, id);
     }
 
     /**
@@ -699,17 +704,78 @@ public class PropertyKey {
      * @return {@link PropertyKey}
      */
     public PropertyKey healthReports(String instanceName) {
-      return new PropertyKey(PropertyType.HEALTHREPORT, HealthStat.class, _clusterName, instanceName);
+      return new PropertyKey(PropertyType.HEALTHREPORT, HealthStat.class, _clusterName,
+          instanceName);
+    }
+
+    /**
+     * Get a PropertyKey associated with root path for Task Framework-related resources' configs.
+     * @return {@link PropertyKey}
+     */
+    public PropertyKey workflowConfigZNodes() {
+      return new PropertyKey(PropertyType.TASK_CONFIG_ROOT, null, _clusterName);
+    }
+
+    /**
+     * Get a PropertyKey associated with root path for Task Framework-related resources' contexts.
+     * @return {@link PropertyKey}
+     */
+    public PropertyKey workflowContextZNodes() {
+      return new PropertyKey(PropertyType.TASK_CONTEXT_ROOT, null, _clusterName);
+    }
+
+    /**
+     * Get a PropertyKey associated with {@link WorkflowConfig} for easier path generation.
+     * @param workflowName
+     * @return {@link PropertyKey}
+     */
+    public PropertyKey workflowConfigZNode(String workflowName) {
+      return new PropertyKey(PropertyType.WORKFLOW_CONFIG, WorkflowConfig.class, _clusterName,
+          workflowName, workflowName);
+    }
+
+    /**
+     * Get a PropertyKey associated with {@link WorkflowConfig} for easier path generation.
+     * @param workflowName
+     * @return {@link PropertyKey}
+     */
+    public PropertyKey workflowContextZNode(String workflowName) {
+      return new PropertyKey(PropertyType.WORKFLOW_CONTEXT, WorkflowConfig.class, _clusterName,
+          workflowName);
+    }
+
+    /**
+     * Get a PropertyKey associated with {@link JobConfig} for easier path generation.
+     * @param workflowName
+     * @param jobName
+     * @return
+     */
+    public PropertyKey jobConfigZNode(String workflowName, String jobName) {
+      return new PropertyKey(PropertyType.JOB_CONFIG, JobConfig.class, _clusterName, workflowName,
+          jobName, jobName);
+    }
+
+    /**
+     * Get a PropertyKey associated with {@link JobContext} for easier path generation.
+     * @param workflowName
+     * @param jobName
+     * @return
+     */
+    public PropertyKey jobContextZNode(String workflowName, String jobName) {
+      return new PropertyKey(PropertyType.JOB_CONTEXT, JobContext.class, _clusterName, workflowName,
+          jobName);
     }
 
     /**
-     * Get a property key associated with {@link WorkflowContext}
-     * TODO: Below must handle the case for future versions of Task Framework with a different path structure
+     * Get a property key associated with {@link WorkflowContext} for easier path generation.
+     * TODO: Below returns the old path for WorkflowContexts
      * @param workflowName
      * @return {@link PropertyKey}
      */
+    @Deprecated
     public PropertyKey workflowContext(String workflowName) {
-      return new PropertyKey(PropertyType.WORKFLOWCONTEXT, WorkflowContext.class, _clusterName, workflowName);
+      return new PropertyKey(PropertyType.WORKFLOWCONTEXT, WorkflowContext.class, _clusterName,
+          workflowName);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/helix/blob/739adb0d/helix-core/src/main/java/org/apache/helix/PropertyPathBuilder.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/PropertyPathBuilder.java b/helix-core/src/main/java/org/apache/helix/PropertyPathBuilder.java
index b0dacdb..e85431c 100644
--- a/helix-core/src/main/java/org/apache/helix/PropertyPathBuilder.java
+++ b/helix-core/src/main/java/org/apache/helix/PropertyPathBuilder.java
@@ -51,6 +51,7 @@ public class PropertyPathBuilder {
 
   static final Map<PropertyType, Map<Integer, String>> templateMap =
       new HashMap<PropertyType, Map<Integer, String>>();
+  @Deprecated // typeToClassMapping is not being used anywhere
   static final Map<PropertyType, Class<? extends HelixProperty>> typeToClassMapping =
       new HashMap<PropertyType, Class<? extends HelixProperty>>();
   static {
@@ -65,7 +66,8 @@ public class PropertyPathBuilder {
     typeToClassMapping.put(HISTORY, LeaderHistory.class);
     typeToClassMapping.put(PAUSE, PauseSignal.class);
     typeToClassMapping.put(MAINTENANCE, MaintenanceSignal.class);
-    // TODO: Below must handle the case for future versions of Task Framework with a different path structure
+    // TODO: Below must handle the case for future versions of Task Framework with a different path
+    // structure
     typeToClassMapping.put(WORKFLOWCONTEXT, WorkflowContext.class);
 
     // @formatter:off
@@ -82,7 +84,8 @@ public class PropertyPathBuilder {
     addEntry(PropertyType.EXTERNALVIEW, 1, "/{clusterName}/EXTERNALVIEW");
     addEntry(PropertyType.EXTERNALVIEW, 2, "/{clusterName}/EXTERNALVIEW/{resourceName}");
     addEntry(PropertyType.TARGETEXTERNALVIEW, 1, "/{clusterName}/TARGETEXTERNALVIEW");
-    addEntry(PropertyType.TARGETEXTERNALVIEW, 2, "/{clusterName}/TARGETEXTERNALVIEW/{resourceName}");
+    addEntry(PropertyType.TARGETEXTERNALVIEW, 2,
+        "/{clusterName}/TARGETEXTERNALVIEW/{resourceName}");
     addEntry(PropertyType.STATEMODELDEFS, 1, "/{clusterName}/STATEMODELDEFS");
     addEntry(PropertyType.STATEMODELDEFS, 2, "/{clusterName}/STATEMODELDEFS/{stateModelName}");
     addEntry(PropertyType.CONTROLLER, 1, "/{clusterName}/CONTROLLER");
@@ -91,14 +94,16 @@ public class PropertyPathBuilder {
     // INSTANCE
     addEntry(PropertyType.MESSAGES, 2, "/{clusterName}/INSTANCES/{instanceName}/MESSAGES");
     addEntry(PropertyType.MESSAGES, 3, "/{clusterName}/INSTANCES/{instanceName}/MESSAGES/{msgId}");
-    addEntry(PropertyType.CURRENTSTATES, 2, "/{clusterName}/INSTANCES/{instanceName}/CURRENTSTATES");
+    addEntry(PropertyType.CURRENTSTATES, 2,
+        "/{clusterName}/INSTANCES/{instanceName}/CURRENTSTATES");
     addEntry(PropertyType.CURRENTSTATES, 3,
         "/{clusterName}/INSTANCES/{instanceName}/CURRENTSTATES/{sessionId}");
     addEntry(PropertyType.CURRENTSTATES, 4,
         "/{clusterName}/INSTANCES/{instanceName}/CURRENTSTATES/{sessionId}/{resourceName}");
     addEntry(PropertyType.CURRENTSTATES, 5,
         "/{clusterName}/INSTANCES/{instanceName}/CURRENTSTATES/{sessionId}/{resourceName}/{bucketName}");
-    addEntry(PropertyType.STATUSUPDATES, 2, "/{clusterName}/INSTANCES/{instanceName}/STATUSUPDATES");
+    addEntry(PropertyType.STATUSUPDATES, 2,
+        "/{clusterName}/INSTANCES/{instanceName}/STATUSUPDATES");
     addEntry(PropertyType.STATUSUPDATES, 3,
         "/{clusterName}/INSTANCES/{instanceName}/STATUSUPDATES/{sessionId}");
     addEntry(PropertyType.STATUSUPDATES, 4,
@@ -111,8 +116,7 @@ public class PropertyPathBuilder {
         "/{clusterName}/INSTANCES/{instanceName}/ERRORS/{sessionId}/{subPath}");
     addEntry(PropertyType.ERRORS, 5,
         "/{clusterName}/INSTANCES/{instanceName}/ERRORS/{sessionId}/{subPath}/{recordName}");
-    addEntry(PropertyType.INSTANCE_HISTORY, 2,
-        "/{clusterName}/INSTANCES/{instanceName}/HISTORY");
+    addEntry(PropertyType.INSTANCE_HISTORY, 2, "/{clusterName}/INSTANCES/{instanceName}/HISTORY");
     addEntry(PropertyType.HEALTHREPORT, 2, "/{clusterName}/INSTANCES/{instanceName}/HEALTHREPORT");
     addEntry(PropertyType.HEALTHREPORT, 3,
         "/{clusterName}/INSTANCES/{instanceName}/HEALTHREPORT/{reportName}");
@@ -133,10 +137,21 @@ public class PropertyPathBuilder {
     // @formatter:on
 
     // RESOURCE
-    // TODO: Below must handle the case for future versions of Task Framework with a different path structure
     addEntry(PropertyType.WORKFLOWCONTEXT, 2,
-        "/{clusterName}/PROPERTYSTORE/TaskRebalancer/{workflowName}/Context");
-
+        "/{clusterName}/PROPERTYSTORE/TaskRebalancer/{workflowName}/Context"); // Old
+                                                                               // WorkflowContext
+                                                                               // path
+    addEntry(PropertyType.TASK_CONFIG_ROOT, 1, "/{clusterName}/CONFIGS/TASK");
+    addEntry(PropertyType.WORKFLOW_CONFIG, 3,
+        "/{clusterName}/CONFIGS/TASK/{workflowName}/{workflowName}");
+    addEntry(PropertyType.JOB_CONFIG, 4,
+        "/{clusterName}/CONFIGS/TASK/{workflowName}/{jobName}/{jobName}");
+    addEntry(PropertyType.TASK_CONTEXT_ROOT, 1,
+        "/{clusterName}/PROPERTYSTORE/TaskFrameworkContext");
+    addEntry(PropertyType.WORKFLOW_CONTEXT, 2,
+        "/{clusterName}/PROPERTYSTORE/TaskFrameworkContext/{workflowName}/Context");
+    addEntry(PropertyType.JOB_CONTEXT, 3,
+        "/{clusterName}/PROPERTYSTORE/TaskFrameworkContext/{workflowName}/{jobName}/Context");
   }
   static Pattern pattern = Pattern.compile("(\\{.+?\\})");
 
@@ -187,8 +202,8 @@ public class PropertyPathBuilder {
       }
     }
     if (result == null || result.indexOf('{') > -1 || result.indexOf('}') > -1) {
-      logger.warn("Unable to instantiate template:" + template + " using clusterName:"
-          + clusterName + " and keys:" + Arrays.toString(keys));
+      logger.warn("Unable to instantiate template:" + template + " using clusterName:" + clusterName
+          + " and keys:" + Arrays.toString(keys));
     }
     return result;
   }
@@ -255,7 +270,8 @@ public class PropertyPathBuilder {
   }
 
   @Deprecated
-  public static String instanceProperty(String clusterName, String instanceName, PropertyType type, String key) {
+  public static String instanceProperty(String clusterName, String instanceName, PropertyType type,
+      String key) {
     return String.format("/%s/INSTANCES/%s/%s/%s", clusterName, instanceName, type, key);
   }
 
@@ -275,23 +291,25 @@ public class PropertyPathBuilder {
     return String.format("/%s/INSTANCES/%s/CURRENTSTATES", clusterName, instanceName);
   }
 
-  public static String instanceCurrentState(String clusterName, String instanceName, String sessionId) {
+  public static String instanceCurrentState(String clusterName, String instanceName,
+      String sessionId) {
     return String.format("/%s/INSTANCES/%s/CURRENTSTATES/%s", clusterName, instanceName, sessionId);
   }
 
-  public static String instanceCurrentState(
-      String clusterName, String instanceName, String sessionId, String resourceName) {
-    return String.format("/%s/INSTANCES/%s/CURRENTSTATES/%s/%s", clusterName, instanceName, sessionId, resourceName);
+  public static String instanceCurrentState(String clusterName, String instanceName,
+      String sessionId, String resourceName) {
+    return String.format("/%s/INSTANCES/%s/CURRENTSTATES/%s/%s", clusterName, instanceName,
+        sessionId, resourceName);
   }
 
   public static String instanceError(String clusterName, String instanceName) {
     return String.format("/%s/INSTANCES/%s/ERRORS", clusterName, instanceName);
   }
 
-  public static String instanceError(
-      String clusterName, String instanceName, String sessionId, String resourceName, String partitionName) {
-    return String.format("/%s/INSTANCES/%s/ERRORS/%s/%s/%s",
-        clusterName, instanceName, sessionId, resourceName, partitionName);
+  public static String instanceError(String clusterName, String instanceName, String sessionId,
+      String resourceName, String partitionName) {
+    return String.format("/%s/INSTANCES/%s/ERRORS/%s/%s/%s", clusterName, instanceName, sessionId,
+        resourceName, partitionName);
   }
 
   public static String instanceHistory(String clusterName, String instanceName) {
@@ -342,7 +360,8 @@ public class PropertyPathBuilder {
     return String.format("/%s/CONTROLLER/STATUSUPDATES", clusterName);
   }
 
-  public static String controllerStatusUpdate(String clusterName, String subPath, String recordName) {
+  public static String controllerStatusUpdate(String clusterName, String subPath,
+      String recordName) {
     return String.format("/%s/CONTROLLER/STATUSUPDATES/%s/%s", clusterName, subPath, recordName);
   }
 

http://git-wip-us.apache.org/repos/asf/helix/blob/739adb0d/helix-core/src/main/java/org/apache/helix/PropertyType.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/PropertyType.java b/helix-core/src/main/java/org/apache/helix/PropertyType.java
index 261d449..2381acc 100644
--- a/helix-core/src/main/java/org/apache/helix/PropertyType.java
+++ b/helix-core/src/main/java/org/apache/helix/PropertyType.java
@@ -64,8 +64,16 @@ public enum PropertyType {
   STATUSUPDATES_CONTROLLER(Type.CONTROLLER, true, true, true),
   ERRORS_CONTROLLER(Type.CONTROLLER, true, true, true),
 
-  // TASK PROPERTY
-  WORKFLOWCONTEXT(Type.TASK, true, false, false, false, false);
+  // TASK PROPERTIES
+  @Deprecated // This returns the old path for WorkflowContext
+  WORKFLOWCONTEXT(Type.TASK, true, false, false, false, false),
+
+  TASK_CONFIG_ROOT(Type.TASK, true, false, false, false, false),
+  TASK_CONTEXT_ROOT(Type.TASK, true, false, false, false, false),
+  WORKFLOW_CONFIG(Type.TASK, true, false, false, false, false),
+  WORKFLOW_CONTEXT(Type.TASK, true, false, false, false, false),
+  JOB_CONFIG(Type.TASK, true, false, false, false, false),
+  JOB_CONTEXT(Type.TASK, true, false, false, false, false);
 
   // @formatter:on
 
@@ -85,26 +93,26 @@ public enum PropertyType {
    */
   boolean isCached;
 
-  private PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate) {
+  PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate) {
     this(type, isPersistent, mergeOnUpdate, false);
   }
 
-  private PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
+  PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
       boolean updateOnlyOnExists) {
     this(type, isPersistent, mergeOnUpdate, false, false);
   }
 
-  private PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
+  PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
       boolean updateOnlyOnExists, boolean createOnlyIfAbsent) {
     this(type, isPersistent, mergeOnUpdate, updateOnlyOnExists, createOnlyIfAbsent, false);
   }
 
-  private PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
+  PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
       boolean updateOnlyOnExists, boolean createOnlyIfAbsent, boolean isCached) {
     this(type, isPersistent, mergeOnUpdate, updateOnlyOnExists, createOnlyIfAbsent, isCached, false);
   }
 
-  private PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
+  PropertyType(Type type, boolean isPersistent, boolean mergeOnUpdate,
       boolean updateOnlyOnExists, boolean createOnlyIfAbsent, boolean isCached, boolean isAsyncWrite) {
     this.type = type;
     this.isPersistent = isPersistent;

http://git-wip-us.apache.org/repos/asf/helix/blob/739adb0d/helix-core/src/test/java/org/apache/helix/util/TestGetWorkflowContext.java
----------------------------------------------------------------------
diff --git a/helix-core/src/test/java/org/apache/helix/util/TestGetWorkflowContext.java b/helix-core/src/test/java/org/apache/helix/util/TestGetWorkflowContext.java
deleted file mode 100644
index 890e6c0..0000000
--- a/helix-core/src/test/java/org/apache/helix/util/TestGetWorkflowContext.java
+++ /dev/null
@@ -1,58 +0,0 @@
-package org.apache.helix.util;
-
-/*
- * 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.
- */
-
-import com.google.common.base.Joiner;
-import org.apache.helix.AccessOption;
-import org.apache.helix.PropertyKey;
-import org.apache.helix.ZNRecord;
-import org.apache.helix.integration.task.TaskTestBase;
-import org.apache.helix.task.TaskConstants;
-import org.apache.helix.task.WorkflowContext;
-import org.testng.Assert;
-import org.testng.annotations.Test;
-
-
-public class TestGetWorkflowContext extends TaskTestBase {
-
-  private static final String WORKFLOW_NAME = "testWorkflow_01";
-  private static final String CONTEXT_NODE = "Context";
-
-  /**
-   * This test method tests whether PropertyKey.Builder successfully creates a path for WorkflowContext instances.
-   * TODO: KeyBuilder must handle the case for future versions of Task Framework with a different path structure
-   */
-  @Test
-  public void testGetWorkflowContext() {
-    // Manually create a WorkflowContext instance
-    ZNRecord znRecord = new ZNRecord(WORKFLOW_NAME);
-    WorkflowContext workflowContext = new WorkflowContext(znRecord);
-    _manager.getHelixPropertyStore().set(
-        Joiner.on("/").join(TaskConstants.REBALANCER_CONTEXT_ROOT, WORKFLOW_NAME, CONTEXT_NODE),
-        workflowContext.getRecord(), AccessOption.PERSISTENT);
-
-    // Test retrieving this WorkflowContext using PropertyKey.Builder.getPath()
-    PropertyKey.Builder keyBuilder = new PropertyKey.Builder(CLUSTER_NAME);
-    String path = keyBuilder.workflowContext(WORKFLOW_NAME).getPath();
-    WorkflowContext workflowCtx = new WorkflowContext(_baseAccessor.get(path, null, AccessOption.PERSISTENT));
-
-    Assert.assertEquals(workflowContext, workflowCtx);
-  }
-}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/helix/blob/739adb0d/helix-core/src/test/java/org/apache/helix/util/TestPropertyKeyGetPath.java
----------------------------------------------------------------------
diff --git a/helix-core/src/test/java/org/apache/helix/util/TestPropertyKeyGetPath.java b/helix-core/src/test/java/org/apache/helix/util/TestPropertyKeyGetPath.java
new file mode 100644
index 0000000..fc0a3f3
--- /dev/null
+++ b/helix-core/src/test/java/org/apache/helix/util/TestPropertyKeyGetPath.java
@@ -0,0 +1,91 @@
+package org.apache.helix.util;
+
+/*
+ * 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.
+ */
+
+import com.google.common.base.Joiner;
+import org.apache.helix.AccessOption;
+import org.apache.helix.PropertyKey;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.integration.task.TaskTestBase;
+import org.apache.helix.task.TaskConstants;
+import org.apache.helix.task.WorkflowContext;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class TestPropertyKeyGetPath extends TaskTestBase {
+
+  private static final String WORKFLOW_NAME = "testWorkflow_01";
+  private static final String JOB_NAME = "testJob_01";
+  private static final String CONFIGS_NODE = "CONFIGS";
+  private static final String TASK_NODE = "TASK";
+  private static final String CONTEXT_NODE = "Context";
+  private static final String TASK_FRAMEWORK_CONTEXT_NODE = "TaskFrameworkContext";
+  private static final String PROPERTYSTORE_NODE = "PROPERTYSTORE";
+  private PropertyKey.Builder KEY_BUILDER = new PropertyKey.Builder(CLUSTER_NAME);
+
+  /**
+   * This test method tests whether PropertyKey.Builder successfully creates a path for
+   * WorkflowContext instances.
+   * TODO: KeyBuilder must handle the case for future versions of Task Framework with a different
+   * path structure
+   */
+  @Test
+  public void testGetWorkflowContext() {
+    // Manually create a WorkflowContext instance
+    ZNRecord znRecord = new ZNRecord(WORKFLOW_NAME);
+    WorkflowContext workflowContext = new WorkflowContext(znRecord);
+    _manager.getHelixPropertyStore().set(
+        Joiner.on("/").join(TaskConstants.REBALANCER_CONTEXT_ROOT, WORKFLOW_NAME, CONTEXT_NODE),
+        workflowContext.getRecord(), AccessOption.PERSISTENT);
+
+    // Test retrieving this WorkflowContext using PropertyKey.Builder.getPath()
+    String path = KEY_BUILDER.workflowContext(WORKFLOW_NAME).getPath();
+    WorkflowContext workflowCtx =
+        new WorkflowContext(_baseAccessor.get(path, null, AccessOption.PERSISTENT));
+
+    Assert.assertEquals(workflowContext, workflowCtx);
+  }
+
+  /**
+   * Tests if the new KeyBuilder APIs for generating workflow and job config/context paths are
+   * working properly.
+   */
+  @Test
+  public void testTaskFrameworkPropertyKeys() {
+    String taskConfigRoot = "/" + Joiner.on("/").join(CLUSTER_NAME, CONFIGS_NODE, TASK_NODE);
+    String workflowConfig = "/"
+        + Joiner.on("/").join(CLUSTER_NAME, CONFIGS_NODE, TASK_NODE, WORKFLOW_NAME, WORKFLOW_NAME);
+    String jobConfig = "/" + Joiner.on("/").join(CLUSTER_NAME, CONFIGS_NODE, TASK_NODE,
+        WORKFLOW_NAME, JOB_NAME, JOB_NAME);
+    String taskContextRoot =
+        "/" + Joiner.on("/").join(CLUSTER_NAME, PROPERTYSTORE_NODE, TASK_FRAMEWORK_CONTEXT_NODE);
+    String workflowContext = "/" + Joiner.on("/").join(CLUSTER_NAME, PROPERTYSTORE_NODE,
+        TASK_FRAMEWORK_CONTEXT_NODE, WORKFLOW_NAME, CONTEXT_NODE);
+    String jobContext = "/" + Joiner.on("/").join(CLUSTER_NAME, PROPERTYSTORE_NODE,
+        TASK_FRAMEWORK_CONTEXT_NODE, WORKFLOW_NAME, JOB_NAME, CONTEXT_NODE);
+
+    Assert.assertEquals(KEY_BUILDER.workflowConfigZNodes().getPath(), taskConfigRoot);
+    Assert.assertEquals(KEY_BUILDER.workflowConfigZNode(WORKFLOW_NAME).getPath(), workflowConfig);
+    Assert.assertEquals(KEY_BUILDER.jobConfigZNode(WORKFLOW_NAME, JOB_NAME).getPath(), jobConfig);
+    Assert.assertEquals(KEY_BUILDER.workflowContextZNodes().getPath(), taskContextRoot);
+    Assert.assertEquals(KEY_BUILDER.workflowContextZNode(WORKFLOW_NAME).getPath(), workflowContext);
+    Assert.assertEquals(KEY_BUILDER.jobContextZNode(WORKFLOW_NAME, JOB_NAME).getPath(), jobContext);
+  }
+}


[2/3] helix git commit: [HELIX-768] TASK: Fix a bug in WorkflowAccessor

Posted by jx...@apache.org.
[HELIX-768] TASK: Fix a bug in WorkflowAccessor

There was a bug in WorkflowAccessor where a JSON was submitted to for PUT requests, it would created a WorkflowConfig based on the JSON but fails to create a workflow with the config. This was preventing users to create workflows via REST APIs properly.
Changelist:
1. Ensure that the configs submitted are reflected in the workflow being created


Project: http://git-wip-us.apache.org/repos/asf/helix/repo
Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/65bb3509
Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/65bb3509
Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/65bb3509

Branch: refs/heads/master
Commit: 65bb35090a90679cc0973d142a1d9a27d1522bbe
Parents: 1d32172
Author: Hunter Lee <hu...@linkedin.com>
Authored: Mon Oct 29 15:54:04 2018 -0700
Committer: Hunter Lee <hu...@linkedin.com>
Committed: Mon Oct 29 15:54:04 2018 -0700

----------------------------------------------------------------------
 .../src/main/java/org/apache/helix/task/WorkflowConfig.java       | 3 +--
 .../helix/rest/server/resources/helix/WorkflowAccessor.java       | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/helix/blob/65bb3509/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java b/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
index 0f8a48c..6136175 100644
--- a/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
+++ b/helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java
@@ -493,8 +493,7 @@ public class WorkflowConfig extends ResourceConfig {
     @Deprecated
     public static Builder fromMap(Map<String, String> cfg) {
       Builder builder = new Builder();
-      builder.setConfigMap(cfg);
-      return builder;
+      return builder.setConfigMap(cfg);
     }
 
     // TODO: Add user customized simple field clone.

http://git-wip-us.apache.org/repos/asf/helix/blob/65bb3509/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/WorkflowAccessor.java
----------------------------------------------------------------------
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/WorkflowAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/WorkflowAccessor.java
index 679b3cc..7cfbe85 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/WorkflowAccessor.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/WorkflowAccessor.java
@@ -138,6 +138,7 @@ public class WorkflowAccessor extends AbstractHelixResource {
       }
 
       Workflow.Builder workflow = new Workflow.Builder(workflowId);
+      workflow.setWorkflowConfig(workflowConfig);
 
       if (root.get(WorkflowProperties.Jobs.name()) != null) {
         Map<String, JobConfig.Builder> jobConfigs =