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 [email protected] or file a JIRA ticket
with INFRA.
---
