You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by iilyak <gi...@git.apache.org> on 2016/08/23 23:05:37 UTC

[GitHub] couchdb-config pull request #12: Add config_listener_mon:start_link/2

GitHub user iilyak opened a pull request:

    https://github.com/apache/couchdb-config/pull/12

    Add config_listener_mon:start_link/2

    This a fixup commit for f09a2eb7d
    
    COUCHDB-3102

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

    $ git pull https://github.com/cloudant/couchdb-config 3102-fixup-for-config_subscription

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

    https://github.com/apache/couchdb-config/pull/12.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 #12
    
----
commit 144da209b5704539dec757a2c2070bb4b6525745
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date:   2016-08-23T23:01:48Z

    Add config_listener_mon:start_link/2
    
    This a fixup commit for f09a2eb7d
    
    COUCHDB-3102

----


---
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] couchdb-config pull request #12: Add config_listener_mon:start_link/2

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

    https://github.com/apache/couchdb-config/pull/12


---
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] couchdb-config issue #12: Add config_listener_mon:start_link/2

Posted by tonysun83 <gi...@git.apache.org>.
Github user tonysun83 commented on the issue:

    https://github.com/apache/couchdb-config/pull/12
  
    +1


---
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] couchdb-config issue #12: Add config_listener_mon:start_link/2

Posted by tonysun83 <gi...@git.apache.org>.
Github user tonysun83 commented on the issue:

    https://github.com/apache/couchdb-config/pull/12
  
    trying to understand this small change in a vacuum. iiuc, originally there were only subscribes which meant we weren't linking to this newly spawned process. now we add a start_link for processes that need to be linked to this process?


---
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] couchdb-config issue #12: Add config_listener_mon:start_link/2

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the issue:

    https://github.com/apache/couchdb-config/pull/12
  
    @tonysun83: We add `start_link` strickly for using the `couch_config_mon` from supervisor.
    Here are the two examples of using the module:
    - using [from a supervisor](https://github.com/apache/couchdb-couch/blob/master/src/couch_sup.erl#L67) - this PR adds that missing `start_link` function.
    - using from [a regular module](https://github.com/apache/couchdb-couch/blob/608942f9ac7c5b2b225ec9598363228583f70f92/src/couch_auth_cache.erl#L156)
    
    As you might notice `listen_for_changes` expected to return `ok`. While `start_link` has to return `{ok, Pid}` to satisfy expectations of supervisor. The reason for adding `start_link` is the fact that supervisor requires a link. We changed `init` to return `{ok, Pid}` in commit f09a2eb.  But that commit missing two important details. It doesn't fix the return value from `listen_for_changes` and it doesn't export `start_link`.


---
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] couchdb-config issue #12: Add config_listener_mon:start_link/2

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the issue:

    https://github.com/apache/couchdb-config/pull/12
  
    When we do `listen_for_changes` we don't establish [a link](https://github.com/apache/couchdb-config/pull/12/files#diff-e603d3168cf3d6b71247b3564c1c21ecR44) but [rely on monitor](https://github.com/cloudant/couchdb-config/blob/144da209b5704539dec757a2c2070bb4b6525745/src/config_listener_mon.erl#L50) instead.
    Having monitor for supervised case (`start_link`) is not required but it wouldn't cause any issues to have 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] couchdb-config issue #12: Add config_listener_mon:start_link/2

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the issue:

    https://github.com/apache/couchdb-config/pull/12
  
    Here is the updates to `rebar.config.script` if you want to run test suite:
    ```
     {couch_epi,        "couch-epi",        "f6ad55d804ac741b59fe37dd092787113847661c"},
    -{config,           "config",           "12dcbf571516970f7cfa586bda9a1f1cad82d5bb"},
    +{config,           "config",           "47e5f7016fd5801ecb647446b1e1a18d42acb0ed"},
     %% keep these sorted
     {b64url,           "b64url",           "6895652d80f95cdf04efb14625abed868998f174"},
    -{couch_log,        "couch-log",        "2f70cc8ddb1a6a8d512aa53490138d9c2754eb07"},
    -{chttpd,           "chttpd",           "663273b0ed2ea4eae16d9d36e1922138fb25e766"},
    -{couch,            "couch",            "1df597fc2e9a8208e7716f8542a7f6a8e9b606e8"},
    -{couch_index,      "couch-index",      "e7d269800302224eacf9585f74eb30822495755b"},
    +{couch_log,        "couch-log",        "ad803f66dbd1900b67543259142875a6d03503ce"},
    +{chttpd,           "chttpd",           "9e0ad343cab5ef652c3e0224da661c4343c146a3"},
    +{couch,            "couch",            "b8b4982f4d591553a5e1cb3bdb56907ee74824ac"},
    +{couch_index,      "couch-index",      "ee21d0181f0879bf9ab2dc6d67cc5c26c7c76628"},
     {couch_mrview,     "couch-mrview",     "157132c6e603b1870a08c04e8459a177387932ed"},
    -{couch_replicator, "couch-replicator", "f67ebff225d63d20ccbcbd33de800f86911a5e79"},
    +{couch_replicator, "couch-replicator", "0248d231d2c9903611d401fc9fc5f9f63960ee7f"},
     {couch_plugins,    "couch-plugins",    "3e73b723cb126cfc471b560d17c24a8b5c540085"},
    -{couch_event,      "couch-event",      "835a41885d1e276d207758954f8238aa7bba0ae8"},
    +{couch_event,      "couch-event",      "7e382132219d708239306aa3591740694943d367"},
     {couch_stats,      "couch-stats",      "7895d4d3f509ed24f09b6d1a0bd0e06af34551dc"},
    -{couch_peruser,    "peruser",          "ff7d190970a46722137fbc7a1a75466e8a544ae1"},
    +{couch_peruser,    "peruser",          "39ef15a28e5be1d697c7c29dd64fcc18c24e7881"},
     {couch_tests,       "erlang-tests",    "37b3bfeb4b1a48a592456e67991362e155ed81e0"},
     {docs,             "documentation",    "59edec50399484ebf0bf784faf28ca6a2f4e8829", [raw]},
     {ddoc_cache,       "ddoc-cache",       "c762e90a33ce3cda19ef142dd1120f1087ecd876"},
    @@ -44,13 +44,13 @@ DepDescs = [
     {fabric,           "fabric",           "78eb096dae277facc5cf4f6f166e5591da6c165f"},
     {fauxton,          "fauxton",          {tag, "v1.1.7"}, [raw]},
     {folsom,           "folsom",           "a5c95dec18227c977029fbd3b638966d98f17003"},
    -{global_changes,   "global-changes",   "203fb088ed81149108e64d18ffd9c3f5df154f97"},
    +{global_changes,   "global-changes",   "f7e43f56ba239f08a3368ff4aea88e50132b7952"},
     {ibrowse,          "ibrowse",          "4af2d408607874d124414ac45df1edbe3961d1cd"},
    -{ioq,              "ioq",              "c7c75ebeaf41599e3a3e211097d864f0e7785829"},
    +{ioq,              "ioq",              "ba99ec70d31cf82fe4868ab4bb92b0978f4fe67f"},
     {jiffy,            "jiffy",            "d3c00e19d8fa20c21758402231247602190988d3"},
     {khash,            "khash",            "7c6a9cd9776b5c6f063ccafedfa984b00877b019"},
     {mango,            "mango",            "01252f971bef0c8da1d78bf5a7b506b71926ce1b"},
    -{mem3,             "mem3",             "15615b295ec970ca9b12b7b54107a80b95149511"},
    +{mem3,             "mem3",             "252467cb4a27637090b5f9006483f5b7ab551699"},
     {mochiweb,         "mochiweb",         "bd6ae7cbb371666a1f68115056f7b30d13765782"},
     {oauth,            "oauth",            "099057a98e41f3aff91e77e3cf496d6c6fd901df"},
     {rexi,             "rexi",             "a327b7dbeb2b0050f7ca9072047bf8ef2d282833"},
    ```
    You would also need to checkout this PR after you've run `./configure`.
    ```
    cd src/config
    git remote add cloudant https://github.com/cloudant/couchdb-config.git
    git fetch cloudant 3102-fixup-for-config_subscription
    git checkout 3102-fixup-for-config_subscription
    ```


---
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] couchdb-config issue #12: Add config_listener_mon:start_link/2

Posted by tonysun83 <gi...@git.apache.org>.
Github user tonysun83 commented on the issue:

    https://github.com/apache/couchdb-config/pull/12
  
    @iilyak : I understand now, `config:listen_for_changes` calls `subscribe` which returns `ok`, whereas the supervisor calls `start_link` and that requires `{ok, Pid}`, and both will link to the process


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