Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
From: wilderrodrigues <git@git.apache.org>
To: dev@cloudstack.apache.org
Reply-To: dev@cloudstack.apache.org
References: <git-pr-238-cloudstack@git.apache.org>
In-Reply-To: <git-pr-238-cloudstack@git.apache.org>
Subject: [GitHub] cloudstack pull request: Build and create tomcat,
 fix several othe...
Content-Type: text/plain
Message-Id: <20150527090512.EE5E6DFF79@git1-us-west.apache.org>
Date: Wed, 27 May 2015 09:05:12 +0000 (UTC)

Github user wilderrodrigues commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/238#discussion_r31114895
  
    --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConsoleProxyLoadCommandWrapper.java ---
    @@ -49,6 +49,11 @@ public Answer executeProxyLoadScan(final Command cmd, final long proxyVmId, fina
                 final URL url = new URL(sb.toString());
                 final URLConnection conn = url.openConnection();
     
    +            // setting TIMEOUTs to avoid possible waiting until death situations
    --- End diff --
    
    Hi @rsafonseca 
    
    Will you remove the changes from you PR and I add to mine, or should we wait for your stuff to get merged?
    
    A couple of question concerning this change:
    
    1. When you run the Unit Tests having this timeout, how do they perform?
    2. Before your changes get through, would you mind replacing the magic number by a static final variable with a clear description of the timeout? It could be added to the LibvirtUtilitiesHelper.
    
    If the tests get somehow slower due to this, we can also add an accessor method to the timeout value. By doing so we can mock the call and when running the tests we can intercept it and return 0, instead of 5000.
    
    Cheers,
    Wilder


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