cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wilderrodrigues <...@git.apache.org>
Subject [GitHub] cloudstack pull request: Build and create tomcat, fix several othe...
Date Wed, 27 May 2015 09:05:12 GMT
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.
---

Mime
View raw message