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).
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #11054 +/- ##
=============================================
- Coverage 15.18% 4.28% -10.90%
=============================================
Files 5416 372 -5044
Lines 475890 29743 -446147
Branches 58093 5229 -52864
=============================================
- Hits 72254 1274 -70980
+ Misses 395550 28324 -367226
+ Partials 8086 145 -7941
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:
|
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?