You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by d2r <gi...@git.apache.org> on 2018/10/12 18:10:42 UTC

[GitHub] storm pull request #2877: STORM-3255: Uses dev-zookeeper in travis-ci

GitHub user d2r opened a pull request:

    https://github.com/apache/storm/pull/2877

    STORM-3255: Uses dev-zookeeper in travis-ci

    Instead of trying to install a zookeeper package that may or may not be available, let's use the same ZooKeeper we ship with storm.

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

    $ git pull https://github.com/d2r/storm storm-3255-use-dev-zk-in-travis

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

    https://github.com/apache/storm/pull/2877.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 #2877
    
----
commit cafe61ffaa98c9656b489b4bc38d1f1689e28535
Author: Derek Dagit <de...@...>
Date:   2018-10-12T18:02:52Z

    Uses dev-zookeeper in travis-ci

----


---

[GitHub] storm pull request #2877: STORM-3255: Uses dev-zookeeper in travis-ci

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

    https://github.com/apache/storm/pull/2877


---

[GitHub] storm issue #2877: STORM-3255: Uses dev-zookeeper in travis-ci

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

    https://github.com/apache/storm/pull/2877
  
    @srdo @revans2 
    This seemed to work on my personal repo.
    I did not bother trying to set maximum client connections to `200` as we did formerly, because we [hard-code](https://github.com/apache/storm/blob/22440747aa0f2ed3f3d26baec9140f79bb82f644/storm-server/src/main/java/org/apache/storm/zookeeper/Zookeeper.java#L87) unlimited connections when we launch `dev-zookeeper`, and I thought this was acceptable.


---

[GitHub] storm pull request #2877: STORM-3255: Uses dev-zookeeper in travis-ci

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

    https://github.com/apache/storm/pull/2877#discussion_r224891745
  
    --- Diff: integration-test/run-it.sh ---
    @@ -91,6 +90,11 @@ function start_storm_process() {
         echo starting: storm $1
         sudo su storm -c "export JAVA_HOME=\"${JAVA_HOME}\" && cd /usr/share/storm && storm $1" &
     }
    +if [[ "${USER}" == 'travis' ]]; then
    +    start_storm_process dev-zookeeper
    --- End diff --
    
    Why not always do this, rather than only on Travis?


---

[GitHub] storm issue #2877: STORM-3255: Uses dev-zookeeper in travis-ci

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

    https://github.com/apache/storm/pull/2877
  
    Looks great, +1


---

[GitHub] storm pull request #2877: STORM-3255: Uses dev-zookeeper in travis-ci

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

    https://github.com/apache/storm/pull/2877#discussion_r224892760
  
    --- Diff: integration-test/run-it.sh ---
    @@ -91,6 +90,11 @@ function start_storm_process() {
         echo starting: storm $1
         sudo su storm -c "export JAVA_HOME=\"${JAVA_HOME}\" && cd /usr/share/storm && storm $1" &
     }
    +if [[ "${USER}" == 'travis' ]]; then
    +    start_storm_process dev-zookeeper
    --- End diff --
    
    I see what appears to be a Vagrant use case, and wanted to scope my change to Travis in case others depended on using a packaged ZK.
    
    We can change the script to do this in any case. I just didn't know what the expectations were. In the vagrant case, the version of the packages is even different (3.4.5 versus 3.4.8).
    
    But I'd be happy to simplify this further if we want.


---

[GitHub] storm pull request #2877: STORM-3255: Uses dev-zookeeper in travis-ci

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

    https://github.com/apache/storm/pull/2877#discussion_r224898629
  
    --- Diff: integration-test/run-it.sh ---
    @@ -91,6 +90,11 @@ function start_storm_process() {
         echo starting: storm $1
         sudo su storm -c "export JAVA_HOME=\"${JAVA_HOME}\" && cd /usr/share/storm && storm $1" &
     }
    +if [[ "${USER}" == 'travis' ]]; then
    +    start_storm_process dev-zookeeper
    --- End diff --
    
    OK this is done.


---