cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rafaelweingartner <...@git.apache.org>
Subject [GitHub] cloudstack pull request: CID-1338387: handle unknown storage endpo...
Date Tue, 10 Nov 2015 18:01:44 GMT
Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/1056#issuecomment-155515107
  
    @ DaanHoogland, I noticed that the select is limiting the RS in one element. That is actually
my point, if the register does not matter, I would rather order by id DESC or ASC and not
at random (of course, I would maintain the limit 1 clause); because that way if something
happens I think it would be easier to track the problem. 
    
    Additionally, as you pointed that out, I think if the host == null, throwing an exception
is a better way to go.
    
    BTW: I was staring at that code now, and something hit me. 
    The method “org.apache.cloudstack.storage.endpoint.DefaultEndPointSelector.selectHypervisorHost(Scope)”
returns an object “EndPoint” that contains a random host id. If the host does not matter,
why not use the host that we have already loaded in line 113? We could just check if the attribute
“hypervisor_type” is not null for that host; that is what the select in “selectHypervisorHost”
method is doing.
    
    I also noticed that you created two new exceptions, would not it be better for us to use
the already known “CloudRuntimeException”? Instead of creating other exception types.
    I think there was a problem with Jenkins, JVM crash ?!
    I will also take a look at PR #1057.



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