You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by fatuhoku <gi...@git.apache.org> on 2014/12/24 21:57:41 UTC

[GitHub] incubator-brooklyn pull request: Couchbase SyncGateway should supp...

GitHub user fatuhoku opened a pull request:

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

    Couchbase SyncGateway should support cluster of ONE node

    `IS_IN_CLUSTER` [is ONLY set when a server is explicitly added into a cluster](https://github.com/apache/incubator-brooklyn/blob/59208f56a264d85d68ccc51bc9f2acf457dc1107/software/nosql/src/main/java/brooklyn/entity/nosql/couchbase/CouchbaseClusterImpl.java#L461-L462).
    
    The original logic checks for 'IS_IN_CLUSTER'; this means that the sole primary node of a single-node cluster will never be eligible to be used with sync gateway (even though it actually is...!).
    
    We extend the boolean condition to check for this condition.

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

    $ git pull https://github.com/fatuhoku/incubator-brooklyn patch-2

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

    https://github.com/apache/incubator-brooklyn/pull/427.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 #427
    
----
commit 72b13293f13d0e39c3c480967ef9b498f20f50e9
Author: Hok Shun Poon <fu...@googlemail.com>
Date:   2014-12-24T20:57:29Z

    Couchbase SyncGateway should support cluster of ONE node
    
    `IS_IN_CLUSTER` [is ONLY set when a server is explicitly added into a cluster](https://github.com/apache/incubator-brooklyn/blob/59208f56a264d85d68ccc51bc9f2acf457dc1107/software/nosql/src/main/java/brooklyn/entity/nosql/couchbase/CouchbaseClusterImpl.java#L461-L462).
    
    The original logic checks for 'IS_IN_CLUSTER'; this means that the sole primary node of a single-node cluster will never be eligible to be used with sync gateway (even though it actually is...!).
    
    We extend the boolean condition to check for this condition.

----


---
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: Couchbase SyncGateway should supp...

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

    https://github.com/apache/incubator-brooklyn/pull/427#issuecomment-68725390
  
    @fatuhoku Thanks for your contribution. Have you committed to an Apache project before? If you haven't you will need to fill out a [contributor's license agreement](http://www.apache.org/licenses/#clas) before we can merge any changes. It's a fairly lightweight thing. 


---
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: Couchbase SyncGateway should supp...

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

    https://github.com/apache/incubator-brooklyn/pull/427#issuecomment-72014883
  
    @fatuhoku was there previously a case where the `entity.IS_PRIMARY_NODE` but not `entity.IS_IN_CLUSTER` ?  that sounds wrong.
    
    just in case it was right, i've merged your change (fixing the minor compile error).
    
    if you could complete a CLA as per @sjcorbett that would be appreciated.  it's not strictly needed for things this small but easier if it is there.
    
    BTW check out update web site on brooklyn.io -- i think you'll find it friendlier than it was


---
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: Couchbase SyncGateway should supp...

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

    https://github.com/apache/incubator-brooklyn/pull/427#issuecomment-72015333
  
    @fatuhoku aha i now see the case where `IS_IN_CLUSTER` isn't being set.  thanks for your code highlight when you opened the PR.  i'l open a new PR to fix that, maybe you could look at it?


---
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: Couchbase SyncGateway should supp...

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

    https://github.com/apache/incubator-brooklyn/pull/427#discussion_r22467236
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/couchbase/CouchbaseSyncGatewaySshDriver.java ---
    @@ -92,10 +92,10 @@ public void launch() {
     
                     @Override
                     public boolean apply(@Nullable Entity entity) {
    -                    if (entity instanceof CouchbaseNode && Boolean.TRUE.equals(entity.getAttribute(CouchbaseNode.IS_IN_CLUSTER))) {
    -                        return true;
    -                    }
    -                    return false;
    +                    // Must either be recognised by a cluster as added, or be the primary node in a cluster of ONE.
    +                    return entity instanceof CouchbaseNode
    +                      && (Boolean.TRUE.equals(entity.getAttribute(entity.IS_IN_CLUSTER))
    +                      || Boolean.TRUE.equals(entity.getAttribute(entity.IS_PRIMARY_NODE)));
    --- End diff --
    
    Replace the 'entity' in `entity.IS_IN_CLUSTER` and `entity.IS_PRIMARY_NODE` with `CouchbaseNode`, otherwise this will not compile.


---
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: Couchbase SyncGateway should supp...

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

    https://github.com/apache/incubator-brooklyn/pull/427#issuecomment-72016315
  
    @fatuhoku should be fixed in https://github.com/apache/incubator-brooklyn/pull/488


---
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: Couchbase SyncGateway should supp...

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

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


---
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.
---