You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by mikezaccardo <gi...@git.apache.org> on 2015/08/05 17:15:29 UTC

[GitHub] incubator-brooklyn pull request: Fix Redis cluster stop logic

GitHub user mikezaccardo opened a pull request:

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

    Fix Redis cluster stop logic

    By catching any exceptions thrown by stopping the slaves or by stopping master, both actions are guaranteed to be attempted.
    
    MultiException is used because it allows us to capture and propagate both exceptions in the event that both the slaves and master fail to stop.

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

    $ git pull https://github.com/mikezaccardo/incubator-brooklyn fix/redis-cluster-stop

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

    https://github.com/apache/incubator-brooklyn/pull/795.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 #795
    
----
commit b65be8ad9eb46d75cdb6fea140f2b98af60882e9
Author: Mike Zaccardo <mi...@cloudsoftcorp.com>
Date:   2015-08-05T15:13:38Z

    Fix logic to ensure that stop will be invoked on both the slaves and master

----


---
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: Fix Redis cluster stop logic

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

    https://github.com/apache/incubator-brooklyn/pull/795#issuecomment-128448005
  
    Must be the renaming -- didn't have conflicts at last commit.  Will fix now.


---
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: Fix Redis cluster stop logic

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

    https://github.com/apache/incubator-brooklyn/pull/795#issuecomment-128430547
  
    Thanks; would merge but it has conflicts. 
    
    @mikezaccardo can you resolve conflicts please? (and then ping folk on IRC to get it merged asap before you hit more conflicts, caused presumably by package renaming!)


---
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: Fix Redis cluster stop logic

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/795#discussion_r36316664
  
    --- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/redis/RedisClusterImpl.java ---
    @@ -119,8 +121,21 @@ public void stop() {
         }
     
         private void doStop() {
    -        getSlaves().invoke(DynamicCluster.STOP, ImmutableMap.<String, Object>of()).getUnchecked();
    -        getMaster().invoke(RedisStore.STOP, ImmutableMap.<String, Object>of()).getUnchecked();
    +        MultiException multiException = new MultiException();
    --- End diff --
    
    Unfortunately can't use `org.mortbay.util` (see asfbot build error). Instead use `brooklyn.util.exceptions.CompoundRuntimeException`.


---
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: Fix Redis cluster stop logic

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

    https://github.com/apache/incubator-brooklyn/pull/795#issuecomment-128786820
  
    Absolutely will do in the future, @richardcloudsoft. I wasn't sure of the best practice so that would have been a good question for the mailing list on my part.


---
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: Fix Redis cluster stop logic

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

    https://github.com/apache/incubator-brooklyn/pull/795#issuecomment-128787877
  
    I went to check and see if that's documented, and it's all there, black and white: https://brooklyn.incubator.apache.org/developers/how-to-contribute.html#contributing-using-github


---
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: Fix Redis cluster stop logic

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

    https://github.com/apache/incubator-brooklyn/pull/795#issuecomment-128778387
  
    Merged. @mikezaccardo our normal process when there's a merge conflict with master is to rebase the branch rather than merge master into it - this avoids ugly `Merge remote-tracking branch 'upstream/master'` messages in the commit history. It's not a major issue here but please look out for this next time :smile: 


---
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: Fix Redis cluster stop logic

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

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


---
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: Fix Redis cluster stop logic

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

    https://github.com/apache/incubator-brooklyn/pull/795#issuecomment-128061198
  
    Fixed, thanks @aledsage


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