You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by mjeelanimsft <gi...@git.apache.org> on 2018/07/20 19:48:52 UTC

[GitHub] zookeeper pull request #579: Connect string fix for non-existent hosts

GitHub user mjeelanimsft opened a pull request:

    https://github.com/apache/zookeeper/pull/579

    Connect string fix for non-existent hosts

    ZKPatch: eda58d9970c76831046ddc45251c9b110856836e (extract)

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

    $ git pull https://github.com/mjeelanimsft/zookeeper connect-string-fix-for-non-existent-hosts

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

    https://github.com/apache/zookeeper/pull/579.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 #579
    
----
commit b3e0cdcef95a1ad043c9a09d8847d0ea0c69361f
Author: Jeelani Mohamed Abdul Khader <mj...@...>
Date:   2018-07-20T18:02:04Z

    Connect string fix for non-existent hosts
    
    ZKPatch: eda58d9970c76831046ddc45251c9b110856836e (extract)

----


---

[GitHub] zookeeper pull request #579: [ZOOKEEPER-3095] Connect string fix for non-exi...

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

    https://github.com/apache/zookeeper/pull/579#discussion_r205666180
  
    --- Diff: src/c/src/zookeeper.c ---
    @@ -842,11 +842,16 @@ static int resolve_hosts(const zhandle_t *zh, const char *hosts_in, addrvec_t *a
             }
     
             freeaddrinfo(res0);
    -
    +next:
             host = strtok_r(0, ",", &strtok_last);
             }
     #endif
         }
    +    if (avec->count == 0) {
    +      rc = ZSYSTEMERROR; // not a single host resolved
    --- End diff --
    
    4 space indent ;)


---

[GitHub] zookeeper pull request #579: [ZOOKEEPER-3095] Connect string fix for non-exi...

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

    https://github.com/apache/zookeeper/pull/579#discussion_r205924496
  
    --- Diff: src/c/tests/zkServer.sh ---
    @@ -77,7 +77,7 @@ fi
     
     if [ "x${base_dir}" == "x" ]
     then
    -zk_base="../../"
    +zk_base="../../../"
    --- End diff --
    
    Yes, seems the old code is not working if the base_dir is not specified. 
    
    We were using this to test the change manually, so it's related from testing purpose. 


---

[GitHub] zookeeper pull request #579: [ZOOKEEPER-3095] Connect string fix for non-exi...

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

    https://github.com/apache/zookeeper/pull/579


---

[GitHub] zookeeper pull request #579: [ZOOKEEPER-3095] Connect string fix for non-exi...

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

    https://github.com/apache/zookeeper/pull/579#discussion_r205666234
  
    --- Diff: src/c/tests/zkServer.sh ---
    @@ -77,7 +77,7 @@ fi
     
     if [ "x${base_dir}" == "x" ]
     then
    -zk_base="../../"
    +zk_base="../../../"
    --- End diff --
    
    is this correct? how did this work before?


---

[GitHub] zookeeper pull request #579: [ZOOKEEPER-3095] Connect string fix for non-exi...

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

    https://github.com/apache/zookeeper/pull/579#discussion_r205666190
  
    --- Diff: src/c/tests/TestClient.cc ---
    @@ -325,7 +326,17 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture
             CPPUNIT_ASSERT(zk != 0);
             CPPUNIT_ASSERT(ctx.waitForConnected(zk));
         }
    -    
    +
    +    /* Checks that a non-existent host will not block the connection*/
    +    void testNonexistentHost() {
    +      char hosts[] = "jimmy:5555,127.0.0.1:22181";
    +      watchctx_t ctx;
    --- End diff --
    
    4 space indent


---

[GitHub] zookeeper issue #579: [ZOOKEEPER-3095] Connect string fix for non-existent h...

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

    https://github.com/apache/zookeeper/pull/579
  
    Thanks for the commit @breed ! My email was indeed not set properly; thanks for letting me know - I've fixed it now.


---

[GitHub] zookeeper issue #579: [ZOOKEEPER-3095] Connect string fix for non-existent h...

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

    https://github.com/apache/zookeeper/pull/579
  
    hey @mjeelanimsft when i did the commit, your email address came up wrong. i think i fixed it, but you might want to check your git config.
    
    thanx for the submission!


---

[GitHub] zookeeper pull request #579: [ZOOKEEPER-3095] Connect string fix for non-exi...

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

    https://github.com/apache/zookeeper/pull/579#discussion_r205666254
  
    --- Diff: src/c/tests/zkServer.sh ---
    @@ -77,7 +77,7 @@ fi
     
     if [ "x${base_dir}" == "x" ]
     then
    -zk_base="../../"
    +zk_base="../../../"
    --- End diff --
    
    and is it related to this change? ;)


---