npe guard for get host info on vmware #11054
Conversation
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
| if (aboutInfo == null) { | ||
| String msg = "no type info about host known" | ||
| s_logger.error(msg) | ||
| throw new Exception(msg); |
There was a problem hiding this comment.
can it be defaulted to VmwareHostType.ESXi ?
There was a problem hiding this comment.
what does it mean if the host does not have this info? Is it in a valid state then?
There was a problem hiding this comment.
I never face this issue before, but I know some users have faced the issue.
If we throw an Exception with message, instead of NPE, I think it does not help, as user will still get an exception.
There was a problem hiding this comment.
yes, but at least it will be clear they have an ill-configured host.
There was a problem hiding this comment.
to be honest, I do not know what is misconfigured and any other issues caused by the misconfiguration.
I think we could consider the host as an ESXi host if the about info is not found.
anyone uses ESX host or other type of host ?
There was a problem hiding this comment.
ok, if y’all insist ;)
There was a problem hiding this comment.
let's get more opinions
@nvazquez @harikrishna-patnala @sureshanaparti @shwstppr @rohityadavcloud
what's your opinion on it ?
There was a problem hiding this comment.
what I meant @weizhouapache , is your argument makes sense, I yield. Please see my changed code fragment at #11054 (comment).
There was a problem hiding this comment.
agreed, assuming ESXi seems to be a good option.
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #11054 +/- ##
============================================
- Coverage 15.17% 15.17% -0.01%
+ Complexity 11363 11357 -6
============================================
Files 5416 5416
Lines 475911 475915 +4
Branches 58096 58097 +1
============================================
- Hits 72241 72229 -12
- Misses 395584 395602 +18
+ Partials 8086 8084 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
DaanHoogland
force-pushed
the
ghi10930-NPEGuard
branch
from
f66ce65 to
d3857c8
Compare
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java
Outdated
Show resolved
Hide resolved
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13884 |
|
@blueorangutan test ol8 vmware-80u3 |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-80u3) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13662)
|
|
[SF] Trillian test result (tid-13700)
|
Description
This PR
Fixes: #10930
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?