You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/08/03 00:43:24 UTC
[GitHub] [helix] rabashizade opened a new pull request #1208: Add DynamicTaskConfig to store task configs in ZK
rabashizade opened a new pull request #1208:
URL: https://github.com/apache/helix/pull/1208
### Issues
- [x] My PR addresses the following Helix issues and references them in the PR description:
Fixes #1207
### Description
- [x] Here are some details about my PR, including screenshots of any UI changes:
This PR adds DynamicTaskConfig class, which is a wrapper for ZNRecord, to store and access the configs for dynamically loaded tasks in ZK. It also adds the appropriate constants that refer to the config fields to TaskConstants.
### Tests
- [ ] The following tests are written for this issue:
- [ ] The following is the result of the "mvn test" command on the appropriate module:
### Commits
- [x] My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation (Optional)
- [ ] In case of new functionality, my PR adds documentation in the following wiki page:
### Code Quality
- [x] My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464529958
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFile() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Set the address of the JAR file containing the task
+ * @param jarFile
+ */
+ public void setJarFile(String jarFile) {
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Set the task version
+ * @param taskVersion
+ */
+ public void seTaskVersion(String taskVersion) {
Review comment:
Typo.
Also, why do you ever need to set the task version after-the-fact?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464768369
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,115 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private final ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFilePath path of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClassesFqns list of the {@link Task} classes fully qualified names
+ * @param taskFactoryFqn {@link TaskFactory} class fully qualified name
+ */
+ public DynamicTaskConfig(String id, String jarFilePath, String taskVersion, List<String> taskClassesFqns,
+ String taskFactoryFqn) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFilePath);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClassesFqns);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactoryFqn);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFilePath() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Get the list of the {@link Task} classes fully qualified names
+ * @return
+ */
+ public List<String> getTaskClassesFqns() {
+ return _taskConfig.getListField(TaskConstants.TASK_CLASSES_KEY);
+ }
+
+ /**
+ * Get the {@link TaskFactory} class fully qualified name
+ * @return
+ */
+ public String getTaskFactoryFqn() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_FACTORY_KEY);
Review comment:
Since we have a TaskFactory - Task Class mapping, do we want a map here instead of two separate lists? How do we match them up otherwise?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464769524
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,115 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private final ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFilePath path of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClassesFqns list of the {@link Task} classes fully qualified names
+ * @param taskFactoryFqn {@link TaskFactory} class fully qualified name
+ */
+ public DynamicTaskConfig(String id, String jarFilePath, String taskVersion, List<String> taskClassesFqns,
+ String taskFactoryFqn) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFilePath);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClassesFqns);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactoryFqn);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFilePath() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Get the list of the {@link Task} classes fully qualified names
+ * @return
+ */
+ public List<String> getTaskClassesFqns() {
+ return _taskConfig.getListField(TaskConstants.TASK_CLASSES_KEY);
+ }
+
+ /**
+ * Get the {@link TaskFactory} class fully qualified name
+ * @return
+ */
+ public String getTaskFactoryFqn() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_FACTORY_KEY);
Review comment:
My reasoning here was that there must be a single TaskFactory, but there could be multiple Task classes (to be invoked depending on the command parameters). So only Task is a list but TaskFactory is a String. Is the assumption flawed?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464208868
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
Review comment:
Is there a reason `DynamicTaskConfig` doesn't use inheritance here?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on pull request #1208:
URL: https://github.com/apache/helix/pull/1208#issuecomment-668711581
This PR is ready to be merged, approved by @narendly
Final commit message:
**Add DynamicTaskConfig to store task configs in ZK**
Adds DynamicTaskConfig class, which is a wrapper for ZNRecord, to store
and access the configs for dynamically loaded tasks in ZK.
Also adds the appropriate constants to TaskConstants.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464529220
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
Review comment:
In that case, this field should be `final`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464439384
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
Review comment:
The reason is that when reading the config using `BaseDataAccessor`, a `ZNRecord` is returned and at runtime a `ClassCastException` is thrown when trying to cast it to `DynamicTaskConfig`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464578270
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFile() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Set the address of the JAR file containing the task
+ * @param jarFile
+ */
+ public void setJarFile(String jarFile) {
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Set the task version
+ * @param taskVersion
+ */
+ public void seTaskVersion(String taskVersion) {
Review comment:
There was a misunderstanding on my part, all the setters should be removed. Thank you for pointing it out.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly merged pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly merged pull request #1208:
URL: https://github.com/apache/helix/pull/1208
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464768496
##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskConstants.java
##########
@@ -49,4 +49,24 @@
public static final String PREV_RA_NODE = "PreviousResourceAssignment";
public static final boolean DEFAULT_TASK_ENABLE_COMPRESSION = false;
+ /**
+ * Name of the JAR file for the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_JAR_FILE_KEY = "JAR_FILE";
+ /**
+ * Version of the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_VERSION_KEY = "VERSION";
+ /**
+ * Name of the {@link Task} class(es) for the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_CLASSES_KEY = "TASK_CLASSES";
+ /**
+ * Name of the {@link TaskFactory} class for the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_FACTORY_KEY = "TASKFACTORY";
Review comment:
Nit: TASK_FACTORY for consistency?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464574185
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
Review comment:
The setter should be removed due to making `_taskConfig` `final`. But the getter is needed to be passed to `create()` in `BaseDataAccessor` when first writing the ZNRecord to ZK.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464535801
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFile() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Set the address of the JAR file containing the task
+ * @param jarFile
+ */
+ public void setJarFile(String jarFile) {
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Set the task version
+ * @param taskVersion
+ */
+ public void seTaskVersion(String taskVersion) {
Review comment:
I think if we want to update the version in the config ZNRecord already stored in ZK, it will be needed.
updateTaskVersion is a better name, I'll change it to better reflect the purpose.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464530519
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
Review comment:
Overall, let's go over this class and make sure we do not have methods (Getters and setters) that we don't/shouldn't need. Think about which fields should be immutable?
Also, let's put a focus on descriptive variable/method names as well.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464529433
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
Review comment:
What are these getters and setters for?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464768369
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,115 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private final ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFilePath path of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClassesFqns list of the {@link Task} classes fully qualified names
+ * @param taskFactoryFqn {@link TaskFactory} class fully qualified name
+ */
+ public DynamicTaskConfig(String id, String jarFilePath, String taskVersion, List<String> taskClassesFqns,
+ String taskFactoryFqn) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFilePath);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClassesFqns);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactoryFqn);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFilePath() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Get the list of the {@link Task} classes fully qualified names
+ * @return
+ */
+ public List<String> getTaskClassesFqns() {
+ return _taskConfig.getListField(TaskConstants.TASK_CLASSES_KEY);
+ }
+
+ /**
+ * Get the {@link TaskFactory} class fully qualified name
+ * @return
+ */
+ public String getTaskFactoryFqn() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_FACTORY_KEY);
Review comment:
Since we have a TaskFactory - Task Class mapping, do we want a map here instead of two separate lists?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464773385
##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskConstants.java
##########
@@ -49,4 +49,24 @@
public static final String PREV_RA_NODE = "PreviousResourceAssignment";
public static final boolean DEFAULT_TASK_ENABLE_COMPRESSION = false;
+ /**
+ * Name of the JAR file for the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_JAR_FILE_KEY = "JAR_FILE";
+ /**
+ * Version of the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_VERSION_KEY = "VERSION";
+ /**
+ * Name of the {@link Task} class(es) for the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_CLASSES_KEY = "TASK_CLASSES";
+ /**
+ * Name of the {@link TaskFactory} class for the task dynamically loaded in {@link TaskStateModel}
+ */
+ public static final String TASK_FACTORY_KEY = "TASKFACTORY";
+ /**
+ * The path for dynamic task configs
+ */
+ public static final String TASK_PATH = "/TASK_DEFINITION";
Review comment:
Let's rename TASK_PATH to DYNAMICALLY_LOADED_TASK_PATH or something. TASK_PATH might add confusion because we actually use /PROPERTYSTORE/TaskRebalancer/
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464529722
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFile() {
Review comment:
JarFilePath? or getLocalJarFilePath? Let's try to use descriptive variable/method names.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464772841
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,115 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private final ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFilePath path of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClassesFqns list of the {@link Task} classes fully qualified names
+ * @param taskFactoryFqn {@link TaskFactory} class fully qualified name
+ */
+ public DynamicTaskConfig(String id, String jarFilePath, String taskVersion, List<String> taskClassesFqns,
+ String taskFactoryFqn) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFilePath);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClassesFqns);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactoryFqn);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFilePath() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Get the list of the {@link Task} classes fully qualified names
+ * @return
+ */
+ public List<String> getTaskClassesFqns() {
+ return _taskConfig.getListField(TaskConstants.TASK_CLASSES_KEY);
+ }
+
+ /**
+ * Get the {@link TaskFactory} class fully qualified name
+ * @return
+ */
+ public String getTaskFactoryFqn() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_FACTORY_KEY);
Review comment:
I see. Thanks for the clarification. In that case, this makes sense. Basically, this factory refers to the factory that can instantiate any implementations of Task.
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,115 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private final ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFilePath path of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClassesFqns list of the {@link Task} classes fully qualified names
+ * @param taskFactoryFqn {@link TaskFactory} class fully qualified name
+ */
+ public DynamicTaskConfig(String id, String jarFilePath, String taskVersion, List<String> taskClassesFqns,
+ String taskFactoryFqn) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFilePath);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClassesFqns);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactoryFqn);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFilePath() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Get the list of the {@link Task} classes fully qualified names
+ * @return
+ */
+ public List<String> getTaskClassesFqns() {
+ return _taskConfig.getListField(TaskConstants.TASK_CLASSES_KEY);
+ }
+
+ /**
+ * Get the {@link TaskFactory} class fully qualified name
+ * @return
+ */
+ public String getTaskFactoryFqn() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_FACTORY_KEY);
Review comment:
I see. Thanks for the clarification. In that case, this makes sense. Basically, this factory refers to **the** factory that can instantiate any implementations of Task.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464535801
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFile() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Set the address of the JAR file containing the task
+ * @param jarFile
+ */
+ public void setJarFile(String jarFile) {
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Set the task version
+ * @param taskVersion
+ */
+ public void seTaskVersion(String taskVersion) {
Review comment:
I think if we want to update the version in the config ZNRecord already stored in ZK, it will be needed.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464768143
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,115 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private final ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFilePath path of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClassesFqns list of the {@link Task} classes fully qualified names
+ * @param taskFactoryFqn {@link TaskFactory} class fully qualified name
+ */
+ public DynamicTaskConfig(String id, String jarFilePath, String taskVersion, List<String> taskClassesFqns,
+ String taskFactoryFqn) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFilePath);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClassesFqns);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactoryFqn);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
Review comment:
Nit: Rename to `getTaskConfigZNRecord`?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464535801
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,155 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFile address of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClasses list of the {@link Task} classes names
+ * @param taskFactory {@link TaskFactory} class name
+ */
+ public DynamicTaskConfig(String id, String jarFile, String taskVersion, List<String> taskClasses,
+ String taskFactory) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClasses);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactory);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Set the task config ZNRecord
+ * @param taskConfig
+ */
+ public void setTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFile() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Set the address of the JAR file containing the task
+ * @param jarFile
+ */
+ public void setJarFile(String jarFile) {
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFile);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Set the task version
+ * @param taskVersion
+ */
+ public void seTaskVersion(String taskVersion) {
Review comment:
I think if we want to update the version in the config ZNRecord already stored in ZK, it will be needed.
updateTaskVersion is a better name, I'll change it to better reflect the purpose.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] rabashizade commented on a change in pull request #1208: Add DynamicTaskConfig to store task configs in ZK
Posted by GitBox <gi...@apache.org>.
rabashizade commented on a change in pull request #1208:
URL: https://github.com/apache/helix/pull/1208#discussion_r464787108
##########
File path: helix-core/src/main/java/org/apache/helix/task/DynamicTaskConfig.java
##########
@@ -0,0 +1,115 @@
+package org.apache.helix.task;
+
+/*
+ * 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 java.util.List;
+
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+
+/**
+ * A wrapper class for ZNRecord, used to store configs for tasks that are to be dynamically loaded
+ */
+public class DynamicTaskConfig {
+ private final ZNRecord _taskConfig;
+
+ /**
+ * Initialize task config with an existing ZNRecord
+ * @param taskConfig
+ */
+ public DynamicTaskConfig(ZNRecord taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ /**
+ * Initialize task config with parameters
+ * @param id
+ * @param jarFilePath path of the JAR file containing the task
+ * @param taskVersion task version
+ * @param taskClassesFqns list of the {@link Task} classes fully qualified names
+ * @param taskFactoryFqn {@link TaskFactory} class fully qualified name
+ */
+ public DynamicTaskConfig(String id, String jarFilePath, String taskVersion, List<String> taskClassesFqns,
+ String taskFactoryFqn) {
+ _taskConfig = new ZNRecord(id);
+ _taskConfig.setSimpleField(TaskConstants.TASK_JAR_FILE_KEY, jarFilePath);
+ _taskConfig.setSimpleField(TaskConstants.TASK_VERSION_KEY, taskVersion);
+ _taskConfig.setListField(TaskConstants.TASK_CLASSES_KEY, taskClassesFqns);
+ _taskConfig.setSimpleField(TaskConstants.TASK_FACTORY_KEY, taskFactoryFqn);
+ }
+
+ /**
+ * Get the task config ZNRecord
+ * @return
+ */
+ public ZNRecord getTaskConfig() {
+ return _taskConfig;
+ }
+
+ /**
+ * Get the address of the JAR file containing the task
+ * @return
+ */
+ public String getJarFilePath() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_JAR_FILE_KEY);
+ }
+
+ /**
+ * Get the task version
+ * @return
+ */
+ public String getTaskVersion() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_VERSION_KEY);
+ }
+
+ /**
+ * Get the list of the {@link Task} classes fully qualified names
+ * @return
+ */
+ public List<String> getTaskClassesFqns() {
+ return _taskConfig.getListField(TaskConstants.TASK_CLASSES_KEY);
+ }
+
+ /**
+ * Get the {@link TaskFactory} class fully qualified name
+ * @return
+ */
+ public String getTaskFactoryFqn() {
+ return _taskConfig.getSimpleField(TaskConstants.TASK_FACTORY_KEY);
Review comment:
Yes, exactly.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org