You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by narendly <gi...@git.apache.org> on 2018/04/05 23:06:19 UTC

[GitHub] helix pull request #174: [HELIX-691] Allow users to update InstanceConfig

GitHub user narendly opened a pull request:

    https://github.com/apache/helix/pull/174

    [HELIX-691] Allow users to update InstanceConfig

    In helix-rest, we provide a method in InstanceAccessor, updateInstanceConfig, that updates the instance's config through a POST call.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/narendly/helix instConfig

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/helix/pull/174.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #174
    
----
commit 1ebdc45194dddecb675362e812aa795c876c9f6a
Author: narendly <na...@...>
Date:   2018-04-05T23:00:52Z

    [HELIX-691] Allow users to update InstanceConfig
    
    In helix-rest, we provide a method in InstanceAccessor, updateInstanceConfig, that updates the instance's config through a POST call.

----


---

[GitHub] helix pull request #174: [HELIX-691] Allow users to update InstanceConfig

Posted by zhan849 <gi...@git.apache.org>.
Github user zhan849 commented on a diff in the pull request:

    https://github.com/apache/helix/pull/174#discussion_r180174778
  
    --- Diff: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ResourceAccessor.java ---
    @@ -84,10 +96,66 @@ public Response getResources(@PathParam("clusterId") String clusterId) {
         return JSONRepresentation(root);
       }
     
    +  /**
    --- End diff --
    
    Partition health related changes are not part of this PR (allow user to change instance config), can we file different issues?


---

[GitHub] helix pull request #174: [HELIX-691] Allow users to update InstanceConfig

Posted by zhan849 <gi...@git.apache.org>.
Github user zhan849 commented on a diff in the pull request:

    https://github.com/apache/helix/pull/174#discussion_r180175528
  
    --- Diff: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java ---
    @@ -223,60 +224,60 @@ public Response updateInstance(@PathParam("clusterId") String clusterId,
           }
     
           switch (cmd) {
    -      case enable:
    -        admin.enableInstance(clusterId, instanceName, true);
    -        break;
    -      case disable:
    -        admin.enableInstance(clusterId, instanceName, false);
    -        break;
    -      case reset:
    -        if (!validInstance(node, instanceName)) {
    -          return badRequest("Instance names are not match!");
    -        }
    -        admin.resetPartition(clusterId, instanceName,
    -            node.get(InstanceProperties.resource.name()).toString(), (List<String>) OBJECT_MAPPER
    -                .readValue(node.get(InstanceProperties.partitions.name()).toString(),
    -                    OBJECT_MAPPER.getTypeFactory()
    -                        .constructCollectionType(List.class, String.class)));
    -        break;
    -      case addInstanceTag:
    -        if (!validInstance(node, instanceName)) {
    -          return badRequest("Instance names are not match!");
    -        }
    -        for (String tag : (List<String>) OBJECT_MAPPER
    -            .readValue(node.get(InstanceProperties.instanceTags.name()).toString(),
    -                OBJECT_MAPPER.getTypeFactory().constructCollectionType(List.class, String.class))) {
    -          admin.addInstanceTag(clusterId, instanceName, tag);
    -        }
    -        break;
    -      case removeInstanceTag:
    -        if (!validInstance(node, instanceName)) {
    -          return badRequest("Instance names are not match!");
    -        }
    -        for (String tag : (List<String>) OBJECT_MAPPER
    -            .readValue(node.get(InstanceProperties.instanceTags.name()).toString(),
    -                OBJECT_MAPPER.getTypeFactory().constructCollectionType(List.class, String.class))) {
    -          admin.removeInstanceTag(clusterId, instanceName, tag);
    -        }
    -        break;
    -      case enablePartitions:
    -        admin.enablePartition(true, clusterId, instanceName,
    -            node.get(InstanceProperties.resource.name()).getTextValue(),
    -            (List<String>) OBJECT_MAPPER
    -                .readValue(node.get(InstanceProperties.partitions.name()).toString(),
    -                    OBJECT_MAPPER.getTypeFactory()
    -                        .constructCollectionType(List.class, String.class)));
    -        break;
    -      case disablePartitions:
    -        admin.enablePartition(false, clusterId, instanceName,
    -            node.get(InstanceProperties.resource.name()).getTextValue(),
    -            (List<String>) OBJECT_MAPPER
    -                .readValue(node.get(InstanceProperties.partitions.name()).toString(),
    -                    OBJECT_MAPPER.getTypeFactory().constructCollectionType(List.class, String.class)));
    -        break;
    -      default:
    -        _logger.error("Unsupported command :" + command);
    -        return badRequest("Unsupported command :" + command);
    +        case enable:
    --- End diff --
    
    Helix's formatter does not indent case, could you pls revert it back? Same for other places


---

[GitHub] helix pull request #174: [HELIX-691] Allow users to update InstanceConfig

Posted by zhan849 <gi...@git.apache.org>.
Github user zhan849 commented on a diff in the pull request:

    https://github.com/apache/helix/pull/174#discussion_r180178181
  
    --- Diff: helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java ---
    @@ -315,26 +316,27 @@ public Response getInstanceConfig(@PathParam("clusterId") String clusterId,
         return notFound();
       }
     
    -  @PUT
    +  @POST
    --- End diff --
    
    PUT is for "set" and POST is for "patch", I'd suggest we keep both. @dasahcc thoughts?


---

[GitHub] helix pull request #174: [HELIX-691] Allow users to update InstanceConfig

Posted by narendly <gi...@git.apache.org>.
Github user narendly closed the pull request at:

    https://github.com/apache/helix/pull/174


---