Github user pnarayanan commented on a diff in the pull request:
https://github.com/apache/helix/pull/65#discussion_r95283285
--- Diff:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java ---
@@ -162,6 +162,20 @@ public InstanceConfig getInstanceConfig(String
clusterName, String instanceName)
}
@Override
+ public boolean setInstanceConfig(String clusterName, String
instanceName,InstanceConfig instanceConfig) {
+ String instanceConfigPath =
+ PropertyPathConfig.getPath(PropertyType.CONFIGS, clusterName,
ConfigScopeProperty.PARTICIPANT.toString(),
+ instanceName);
+ if (!_zkClient.exists(instanceConfigPath)) {
+ throw new HelixException("instance" + instanceName + " does not
exist in cluster " + clusterName);
+ }
+
+ HelixDataAccessor accessor = new ZKHelixDataAccessor(clusterName, new
ZkBaseDataAccessor<ZNRecord>(_zkClient));
+ Builder keyBuilder = accessor.keyBuilder();
+ return accessor.setProperty(keyBuilder.instanceConfig(instanceName),
instanceConfig);
--- End diff --
`updateInstanceConfig()` is what I initially had, but merging wasn't what I
wanted. Basically, I was looking for a way to remove elements from a List field
in the config - and merging always appends new elements to the list, never
removes from it. Is there something I am missing?
I agree with your first suggestion. I think we can throw an exception if
those two fields are not the same between the old instanceConfig and the new
one to make the API tight (rather than silently setting it from the old one).
What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
