You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by bostko <gi...@git.apache.org> on 2015/04/01 12:56:31 UTC

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

GitHub user bostko opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/581

    Riak - Create Bucket type effector

    This is the initial pull request for supporting riak bucket types in brooklyn.
    http://docs.basho.com/riak/latest/dev/advanced/bucket-types/
    
    Bucket types are more used for dynamic configurations, for example changing easily properties of a running cluster.
    Riak supports adding, updating and deleting bucket types.
    
    I added an effector which creates a bucket type.
    I still can not figure out how can we add some monitoring /sensors/ to those bucket types?

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

    $ git pull https://github.com/bostko/incubator-brooklyn riak-bucket-types

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

    https://github.com/apache/incubator-brooklyn/pull/581.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 #581
    
----
commit f04c3ad8c5421ad4870b2e7dcff5e9de976c4492
Author: Valentin Aitken <bo...@gmail.com>
Date:   2015-04-01T10:48:40Z

    Riak - Create Bucket type effector

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27612386
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,21 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    void commitCluster();
     
    +    @Effector(description = "Apply bucket type")
    +    void applyBucketType(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    --- End diff --
    
    Is it feasible to have an integration and/or live test that uses this (including subsequently using the bucket-type that is created + activated)?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27929036
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -395,12 +390,12 @@ public void joinCluster(String nodeName) {
                 if (!hasJoinedCluster()) {
                     ScriptHelper joinClusterScript = newScript("joinCluster")
                             .body.append(sudo(format("%s cluster join %s", getRiakAdminCmd(), nodeName)))
    +                        .body.append(sudo(format("%s cluster plan", getRiakAdminCmd())))
    +                        .body.append(sudo(format("%s cluster commit", getRiakAdminCmd())))
    --- End diff --
    
    Actually I found why it is implemented that way.
    I think the initial idea was to support cluster queue operations. It is correct not to commit just after the cluster join command. Other than that it was executed for one of the members.
    Instead of that commit was executed after the joinings are done and then in the RiakCluster.start at the end it was executed clusterCommit()
    The bug here is that the commit is not executed properly because every time joining was executed against random member of the ring instead of using one, so commiting should be done against the nodes or the node which triggered cluster join. This is the actual bug. And the other bug is may be that leaveCluster had plan and commit command which is unnecessary.
    
    My point is that it could support queue cluster operations but the task requires bigger effort.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877246
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    +    void bucketTypeCreate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    +                          @EffectorParam(name = "bucketTypeProperties") String bucketTypeProperties);
     
    +    @Effector(description = "List bucket types")
    +    List<String> bucketTypeList();
    --- End diff --
    
    as above, call this `listBucketTypes` ?  (note the plural.)  and expand the description, at least a little.  `List the names all bucket types in the system.`


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27869223
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -542,4 +602,10 @@ public String getOsMajorVersion() {
             String osVersion = osDetails.getVersion();
             return osVersion.contains(".") ? osVersion.substring(0, osVersion.indexOf(".")) : osVersion;
         }
    +
    +    private void addRiakOnPath(ScriptHelper scriptHelper) {
    +        Map<String, String> newPathVariable = ImmutableMap.of("PATH", sbinPath);
    +//        log.warn("riak command not found on PATH. Altering future commands' environment variables from {} to {}", getShellEnvironment(), newPathVariable);
    --- End diff --
    
    Why comment it out? If really not needed better remove it altogether.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-90556650
  
    looks good.  add the TODO and expand descriptions, then i think good to merge.
    
    can test whether we can abandon `sudo` in a subsequent PR.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877281
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    +    void bucketTypeCreate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    +                          @EffectorParam(name = "bucketTypeProperties") String bucketTypeProperties);
     
    +    @Effector(description = "List bucket types")
    +    List<String> bucketTypeList();
    +
    +    @Effector(description = "Status bucket type")
    +    List<String> bucketTypeStatus(@EffectorParam(name = "bucketTypeName") String bucketTypeName);
    --- End diff --
    
    don't understand this -- is it `lookupBucketTypeStatus` ?  what is the meaning of the return `List<String>` ?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877711
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    +    void bucketTypeCreate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    --- End diff --
    
    as discussed -- you are following the riak CLI syntax, with nice side effect that effectors are nicely sorted.  makes a lot of sense.  disregard these comments.  :)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877308
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    +    void bucketTypeCreate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    +                          @EffectorParam(name = "bucketTypeProperties") String bucketTypeProperties);
     
    +    @Effector(description = "List bucket types")
    +    List<String> bucketTypeList();
    +
    +    @Effector(description = "Status bucket type")
    +    List<String> bucketTypeStatus(@EffectorParam(name = "bucketTypeName") String bucketTypeName);
    +
    +    @Effector(description = "Update bucket type")
    +    void bucketTypeUpdate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    +                          @EffectorParam(name = "bucketTypeProperties") String bucketTypeProperties);
    +
    +    @Effector(description = "Activate bucket type")
    +    void bucketTypeActivate(@EffectorParam(name = "bucketTypeName") String bucketTypeName);
    --- End diff --
    
    and again


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27869514
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -474,6 +463,79 @@ public void commitCluster() {
         }
     
         @Override
    +    public void bucketTypeCreate(String bucketTypeName, String bucketTypeProperties) {
    +        ScriptHelper bucketTypeCreateScript = newScript("bucket-type_create " + bucketTypeName)
    +                .body.append(sudo(format("%s bucket-type create %s %s",
    +                        getRiakAdminCmd(),
    +                        bucketTypeName,
    +                        escapeLiteralForDoubleQuotedBash(bucketTypeProperties))));
    +        if(!isRiakOnPath()) {
    +            addRiakOnPath(bucketTypeCreateScript);
    +        }
    +        bucketTypeCreateScript.body.append(sudo(format("%s bucket-type activate %s", getRiakAdminCmd(), bucketTypeName)))
    +                .failOnNonZeroResultCode();
    +
    +        bucketTypeCreateScript.execute();
    +    }
    +
    +    @Override
    +    public List<String> bucketTypeList() {
    +        ScriptHelper bucketTypeListScript = newScript("bucket-types_list")
    +                .body.append(sudo(format("%s bucket-type list", getRiakAdminCmd())))
    +                .gatherOutput()
    +                .noExtraOutput()
    +                .failOnNonZeroResultCode();
    +        if (!isRiakOnPath()) {
    +            addRiakOnPath(bucketTypeListScript);
    +        }
    +        bucketTypeListScript.execute();
    +        String stdout = bucketTypeListScript.getResultStdout();
    +        return Arrays.asList(stdout.split("[\\r\\n]+"));
    +    }
    +
    +    @Override
    +    public List<String> bucketTypeStatus(String bucketTypeName) {
    +        ScriptHelper bucketTypeStatusScript = newScript("bucket-type_status")
    +                .body.append(sudo(format("%s bucket-type status %s", getRiakAdminCmd(), bucketTypeName)))
    +                .gatherOutput()
    +                .noExtraOutput()
    +                .failOnNonZeroResultCode();
    +        if (!isRiakOnPath()) {
    +            addRiakOnPath(bucketTypeStatusScript);
    +        }
    +        bucketTypeStatusScript.execute();
    +        String stdout = bucketTypeStatusScript.getResultStdout();
    +        return Arrays.asList(stdout.split("[\\r\\n]+"));
    +    }
    +
    +    @Override
    +    public void bucketTypeUpdate(String bucketTypeName, String bucketTypeProperties) {
    +        ScriptHelper bucketTypeStatusScript = newScript("bucket-type_update")
    +                .body.append(sudo(format("%s bucket-type update %s %s",
    +                        getRiakAdminCmd(),
    +                        bucketTypeName,
    +                        escapeLiteralForDoubleQuotedBash(bucketTypeProperties))))
    +                .noExtraOutput()
    --- End diff --
    
    You are right


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877407
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -445,32 +438,81 @@ public void removeNode(String nodeName) {
                     .failOnNonZeroResultCode();
     
             if (!isRiakOnPath()) {
    -            Map<String, String> newPathVariable = ImmutableMap.of("PATH", sbinPath);
    -            log.warn("riak command not found on PATH. Altering future commands' environment variables from {} to {}", getShellEnvironment(), newPathVariable);
    -            removeNodeScript.environmentVariablesReset(newPathVariable);
    +            addRiakOnPath(removeNodeScript);
             }
     
             removeNodeScript.execute();
         }
     
         @Override
    -    public void commitCluster() {
    -        if (hasJoinedCluster()) {
    -            ScriptHelper commitClusterScript = newScript("commitCluster")
    -                    .body.append(sudo(format("%s cluster plan", getRiakAdminCmd())))
    -                    .body.append(sudo(format("%s cluster commit", getRiakAdminCmd())))
    -                    .failOnNonZeroResultCode();
    +    public void bucketTypeCreate(String bucketTypeName, String bucketTypeProperties) {
    --- End diff --
    
    again, change method names here to match effector rename


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877188
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    --- End diff --
    
    i hate descriptions and javadoc which simply restate the name.  say `Creates a new bucket type with the given name and properties, with properties in <?what kind of ?> format`.  (or should properties be a map?)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-90549818
  
    btw @bostko to answer your question in original post:  if we want sensors/effectors for individual bucket types what i would do is add entities for those bucket types (and same for buckets, and for databases in DBMSs etc).
    
    not needed at this stage.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27874342
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -395,12 +390,12 @@ public void joinCluster(String nodeName) {
                 if (!hasJoinedCluster()) {
                     ScriptHelper joinClusterScript = newScript("joinCluster")
                             .body.append(sudo(format("%s cluster join %s", getRiakAdminCmd(), nodeName)))
    +                        .body.append(sudo(format("%s cluster plan", getRiakAdminCmd())))
    +                        .body.append(sudo(format("%s cluster commit", getRiakAdminCmd())))
    --- End diff --
    
    Very good note.
    The point here is that this clusterCommit() method is made as effector.
    If we go to the use case of that command it is as following: executing one of the following effectors: leaveCluster(), joinCluster(), removeNode() and maybe recoverFailedNode()
    So let's say we want to support executing these commands and then invoking commitCluster() at the end.
    
    However when setting up a new cluster blueprint it will lead that at the end of blueprint initialization the user should manually go to the UI and click on commitCluster()
    The way we could workaround this is by introducing new version of the methods leaveCluster(), joinCluster(), removeNode() like leaveClusterEffector(), joinClusterEffector(), removeNodeEffector() which is without invoking commitCluster() and leave the current methods as they are.
    That way deploying and operations queue would work but then it won't be very well supported in the UI, it will be easy for the user to miss to invoke commitCluster().


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877300
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    +    void bucketTypeCreate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    +                          @EffectorParam(name = "bucketTypeProperties") String bucketTypeProperties);
     
    +    @Effector(description = "List bucket types")
    +    List<String> bucketTypeList();
    +
    +    @Effector(description = "Status bucket type")
    +    List<String> bucketTypeStatus(@EffectorParam(name = "bucketTypeName") String bucketTypeName);
    +
    +    @Effector(description = "Update bucket type")
    +    void bucketTypeUpdate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    --- End diff --
    
    verb first again


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877205
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    +    void bucketTypeCreate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    --- End diff --
    
    I am following the riak commands flow.
    This way it will also stay sorted in the UI, so bucket stuff will be next to each other


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-90856428
  
    LGTM.  Needs a lot of testing, but going ahead and merging.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27868963
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -395,12 +390,12 @@ public void joinCluster(String nodeName) {
                 if (!hasJoinedCluster()) {
                     ScriptHelper joinClusterScript = newScript("joinCluster")
                             .body.append(sudo(format("%s cluster join %s", getRiakAdminCmd(), nodeName)))
    +                        .body.append(sudo(format("%s cluster plan", getRiakAdminCmd())))
    +                        .body.append(sudo(format("%s cluster commit", getRiakAdminCmd())))
    --- End diff --
    
    Looking at the driver API looks like .commitCluster() is meant to be called as a separate command. Why call commit here and not for the other methods?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-89426994
  
    I included also a fix for the cluster join command
    http://docs.basho.com/riak/latest/ops/building/basic-cluster-setup/#Add-a-Second-Node-to-Your-Cluster


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-88633951
  
    @bostko not sure what has happened to this PR - github thinks it includes the two commits that have been merged to master in other pull requests! Out of interest, how did you `git pull` or `git rebase` your branch?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27869493
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -542,4 +602,10 @@ public String getOsMajorVersion() {
             String osVersion = osDetails.getVersion();
             return osVersion.contains(".") ? osVersion.substring(0, osVersion.indexOf(".")) : osVersion;
         }
    +
    +    private void addRiakOnPath(ScriptHelper scriptHelper) {
    +        Map<String, String> newPathVariable = ImmutableMap.of("PATH", sbinPath);
    +//        log.warn("riak command not found on PATH. Altering future commands' environment variables from {} to {}", getShellEnvironment(), newPathVariable);
    --- End diff --
    
    For distributions that require this, It is annoying to show it all the time in the log


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27877074
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
    @@ -183,18 +183,31 @@
         String getOsMajorVersion();
     
         @Effector(description = "Join the Riak cluster on the given node")
    -    public void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
    +    void joinCluster(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Leave the Riak cluster")
    -    public void leaveCluster();
    +    void leaveCluster();
     
         @Effector(description = "Remove the given node from the Riak cluster")
    -    public void removeNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void removeNode(@EffectorParam(name = "nodeName") String nodeName);
     
         @Effector(description = "Recover and join the Riak cluster on the given node")
    -    public void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
    +    void recoverFailedNode(@EffectorParam(name = "nodeName") String nodeName);
     
    -    @Effector(description = "Commit changes made to the Riak cluster")
    -    public void commitCluster();
    +    @Effector(description = "Create bucket type")
    +    void bucketTypeCreate(@EffectorParam(name = "bucketTypeName") String bucketTypeName,
    --- End diff --
    
    verb first is the usual pattern for methods isn't it?   so `createBucketType`


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-90579624
  
    I fixed the bucket type effectors' description and I added a TODO in https://github.com/bostko/incubator-brooklyn/commit/343ed278035e4fe58d9273add9c4f16076d09a24


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-88634802
  
    Thank you for your comment.
    It is still not ready for merging I just wanted some feedback


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27869144
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -474,6 +463,79 @@ public void commitCluster() {
         }
     
         @Override
    +    public void bucketTypeCreate(String bucketTypeName, String bucketTypeProperties) {
    +        ScriptHelper bucketTypeCreateScript = newScript("bucket-type_create " + bucketTypeName)
    +                .body.append(sudo(format("%s bucket-type create %s %s",
    +                        getRiakAdminCmd(),
    +                        bucketTypeName,
    +                        escapeLiteralForDoubleQuotedBash(bucketTypeProperties))));
    +        if(!isRiakOnPath()) {
    +            addRiakOnPath(bucketTypeCreateScript);
    +        }
    +        bucketTypeCreateScript.body.append(sudo(format("%s bucket-type activate %s", getRiakAdminCmd(), bucketTypeName)))
    +                .failOnNonZeroResultCode();
    +
    +        bucketTypeCreateScript.execute();
    +    }
    +
    +    @Override
    +    public List<String> bucketTypeList() {
    +        ScriptHelper bucketTypeListScript = newScript("bucket-types_list")
    +                .body.append(sudo(format("%s bucket-type list", getRiakAdminCmd())))
    +                .gatherOutput()
    +                .noExtraOutput()
    +                .failOnNonZeroResultCode();
    +        if (!isRiakOnPath()) {
    +            addRiakOnPath(bucketTypeListScript);
    +        }
    +        bucketTypeListScript.execute();
    +        String stdout = bucketTypeListScript.getResultStdout();
    +        return Arrays.asList(stdout.split("[\\r\\n]+"));
    +    }
    +
    +    @Override
    +    public List<String> bucketTypeStatus(String bucketTypeName) {
    +        ScriptHelper bucketTypeStatusScript = newScript("bucket-type_status")
    +                .body.append(sudo(format("%s bucket-type status %s", getRiakAdminCmd(), bucketTypeName)))
    +                .gatherOutput()
    +                .noExtraOutput()
    +                .failOnNonZeroResultCode();
    +        if (!isRiakOnPath()) {
    +            addRiakOnPath(bucketTypeStatusScript);
    +        }
    +        bucketTypeStatusScript.execute();
    +        String stdout = bucketTypeStatusScript.getResultStdout();
    +        return Arrays.asList(stdout.split("[\\r\\n]+"));
    +    }
    +
    +    @Override
    +    public void bucketTypeUpdate(String bucketTypeName, String bucketTypeProperties) {
    +        ScriptHelper bucketTypeStatusScript = newScript("bucket-type_update")
    +                .body.append(sudo(format("%s bucket-type update %s %s",
    +                        getRiakAdminCmd(),
    +                        bucketTypeName,
    +                        escapeLiteralForDoubleQuotedBash(bucketTypeProperties))))
    +                .noExtraOutput()
    --- End diff --
    
    no need for `noExtraOutput` if you are not parsing stdout


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27612475
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -474,6 +475,18 @@ public void commitCluster() {
         }
     
         @Override
    +    public void applyBucketType(String bucketTypeName, String bucketTypeProperties) {
    --- End diff --
    
    perhaps "createBucketType" would be a better method name, given that we all Riak's `bucket-type create ...`? (even though it means create or modify - still arguably better to be consistent with Riak's commands and documentation). Same goes for effector method on RiakNode as well (but the effector description can say "Create or modify a bucket type before activation" [1]).
    
    Do we also need to activate the bucket type: "Like all bucket types, this type needs to be activated to be usable within the cluster." [1]. That is, call `riak-admin bucket-type activate ...`?
    
    [1] http://docs.basho.com/riak/latest/dev/advanced/bucket-types/


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/581#issuecomment-90511569
  
    Brooklyn side changes looking good, can't comment on the riak commands.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Riak - Create Bucket type effecto...

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

    https://github.com/apache/incubator-brooklyn/pull/581#discussion_r27876100
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
    @@ -395,12 +390,12 @@ public void joinCluster(String nodeName) {
                 if (!hasJoinedCluster()) {
                     ScriptHelper joinClusterScript = newScript("joinCluster")
                             .body.append(sudo(format("%s cluster join %s", getRiakAdminCmd(), nodeName)))
    +                        .body.append(sudo(format("%s cluster plan", getRiakAdminCmd())))
    +                        .body.append(sudo(format("%s cluster commit", getRiakAdminCmd())))
    --- End diff --
    
    agree, it feels like our default behaviour should be to commit.
    
    we could (in time) add an optional parameter "commitChange" which defaults to true, that way if someone wanted to they could batch/preview changes.
    
    i'd say add a TODO with this (and reference the commit which includes the commitCluster effector, in case we want to revive that we can copy-paste from here.)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---