You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by akira <gi...@git.apache.org> on 2017/02/10 21:56:04 UTC
[GitHub] curator pull request #198: [CURATOR-386] Allow listener to be passed in to P...
GitHub user akira opened a pull request:
https://github.com/apache/curator/pull/198
[CURATOR-386] Allow listener to be passed in to PersistentNode
This provides ability to hook into events from PersistentNode when a
node gets created. PersistentNode would then notify any registered listeners that the
node was created, along with the path created (useful when protection is used).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/akira/curator CURATOR-386
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/curator/pull/198.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 #198
----
commit 8bb0cc86343eca7e2b38bccda39ab28d6c19113c
Author: Alex Kira <ak...@uber.com>
Date: 2017-02-10T21:52:59Z
[CURATOR-386] Allow listener to be passed in to PersistentNode
This provides ability to hook into events from PersistentNode when a
node gets created.
----
---
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] curator issue #198: [CURATOR-386] Allow listener to be passed in to Persiste...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/198
Thanks @akira, the new patch looks good to me.
---
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] curator issue #198: [CURATOR-386] Allow listener to be passed in to Persiste...
Posted by akira <gi...@git.apache.org>.
Github user akira commented on the issue:
https://github.com/apache/curator/pull/198
Ran tests locally:
```
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Apache Curator ..................................... SUCCESS [ 3.114 s]
[INFO] Curator Testing .................................... SUCCESS [ 8.040 s]
[INFO] Curator Client ..................................... SUCCESS [ 37.217 s]
[INFO] Curator Framework .................................. SUCCESS [09:36 min]
[INFO] Curator Recipes .................................... SUCCESS [29:43 min]
[INFO] Curator Service Discovery .......................... SUCCESS [01:26 min]
[INFO] Curator Examples ................................... SUCCESS [ 2.376 s]
[INFO] Curator Service Discovery Server ................... SUCCESS [ 10.766 s]
[INFO] Curator RPC Proxy .................................. SUCCESS [ 13.581 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
```
---
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] curator issue #198: [CURATOR-386] Allow listener to be passed in to Persiste...
Posted by akira <gi...@git.apache.org>.
Github user akira commented on the issue:
https://github.com/apache/curator/pull/198
@lvfangmin thanks, when do you think it will 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] curator issue #198: [CURATOR-386] Allow listener to be passed in to Persiste...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:
https://github.com/apache/curator/pull/198
@akira sorry for the lately reply, I was waiting for another LGTM, the tests look good on my Mac, I'm going to merge the code 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] curator pull request #198: [CURATOR-386] Allow listener to be passed in to P...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/curator/pull/198
---
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] curator pull request #198: [CURATOR-386] Allow listener to be passed in to P...
Posted by akira <gi...@git.apache.org>.
Github user akira commented on a diff in the pull request:
https://github.com/apache/curator/pull/198#discussion_r106834514
--- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java ---
@@ -117,6 +120,7 @@ public void processResult(CuratorFramework client, CuratorEvent event)
{
//Update is ok, mark initialisation as complete if required.
initialisationComplete();
+ notifyListeners();
--- End diff --
@lvfangmin I looked at this - and it looks to me like this is the callback where the result of node creation comes back (telling us whether it was successful, node already exists, etc). This is in the `processBackgroundCallback` method, and where we know that the result code comes back as `false` for `KeeperException.Code.NODEEXISTS` and `true` for `KeeperException.Code.OK`. This means that we just created the node, and seemingly the place to notify listeners.
The callback for updating the node value seems to be `setDataCallback`, which I did not put any code for notifying listeners.
To me this looked like the place to put this, but definitely let me know if you thought of a better place and I can investigate that.
---
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] curator pull request #198: [CURATOR-386] Allow listener to be passed in to P...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/curator/pull/198#discussion_r105545812
--- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java ---
@@ -117,6 +120,7 @@ public void processResult(CuratorFramework client, CuratorEvent event)
{
//Update is ok, mark initialisation as complete if required.
initialisationComplete();
+ notifyListeners();
--- End diff --
the listener is supposed to listen on the node create event, this is the place where we updated the node value, not create.
---
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] curator issue #198: [CURATOR-386] Allow listener to be passed in to Persiste...
Posted by akira <gi...@git.apache.org>.
Github user akira commented on the issue:
https://github.com/apache/curator/pull/198
@lvfangmin I updated the PR based on your feedback - moved the notifyListeners to the `processBackgroundCallback` callback (I squashed the commit with the previous one).
---
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.
---