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

[GitHub] brooklyn-server pull request: Fix BROOKLYN-268: DynamicCluster on-...

GitHub user aledsage opened a pull request:

    https://github.com/apache/brooklyn-server/pull/140

    Fix BROOKLYN-268: DynamicCluster on-fire when initially empty

    Fixes `DynamicClusterTest.testServiceUpAfterStartingWithNoMembers`

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

    $ git pull https://github.com/aledsage/brooklyn-server BROOKLYN-268

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

    https://github.com/apache/brooklyn-server/pull/140.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 #140
    
----
commit 293448c6cf84b7842640b6c68253ef8c0df976d6
Author: Aled Sage <al...@gmail.com>
Date:   2016-05-17T14:23:18Z

    Fix BROOKLYN-268: DynamicCluster on-fire when initially empty

----


---
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] brooklyn-server pull request: Fix BROOKLYN-268: DynamicCluster on-...

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

    https://github.com/apache/brooklyn-server/pull/140#issuecomment-220049819
  
    I see. Thanks for the explanation, @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.
---

[GitHub] brooklyn-server pull request: Fix BROOKLYN-268: DynamicCluster on-...

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

    https://github.com/apache/brooklyn-server/pull/140#issuecomment-220034504
  
    @kiuby88 unfortunately the test seems to be non-deterministic (without this fix). That is presumably because there is a race between the enricher setting the service.isUp to null, and DynamicClusterImpl explicitly setting the service.isUp to true.
    
    Thanks for reviewing!
    
    @ahgittin any comments on this (and the subtleties of the `ServiceStateLogic`)? Or is this good to merge?


---
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] brooklyn-server pull request: Fix BROOKLYN-268: DynamicCluster on-...

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

    https://github.com/apache/brooklyn-server/pull/140#issuecomment-220152500
  
    Merging this now, to fix the failing test. @ahgittin if you have any subsequent comments then please do let me know.


---
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] brooklyn-server pull request: Fix BROOKLYN-268: DynamicCluster on-...

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

    https://github.com/apache/brooklyn-server/pull/140#issuecomment-219827742
  
    Hi @aledsage 
    I have tried to reproduce the issue using OS X 10.9.5, Ubuntu 14.04.3 LTS and 12.04.05 LTS all of then with maven 3.3.9 and jdk 1.7 but I was not lucky, I cannot reproduce the error. However, I find the issue that you mentioned in [Jenkins' report](https://builds.apache.org/job/brooklyn-server-pull-requests/393/) of #141. I am a little confused because we should find the same error in the last PRs, for example in #142 where `DynamicClusterImpl` does not include `sensors().set(ServiceStateLogic.SERVICE_NOT_UP_INDICATORS, MutableMap.<String, Object>of());`. WDYT? May the issue be in a different part?
    
    In any case I have also tested this PR in the aforementioned OO.SS and it passes all the tests. So, it could be merged.


---
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] brooklyn-server pull request: Fix BROOKLYN-268: DynamicCluster on-...

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

    https://github.com/apache/brooklyn-server/pull/140


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