alexandru-bagu edited a comment on pull request #4516:
URL: https://github.com/apache/cloudstack/pull/4516#issuecomment-740807443


   > > > @alexandru-bagu why _host.getHypervisorType()_ is null in your case, 
here:
   > > > 
https://github.com/apache/cloudstack/blob/fb1e903532d943bc98a1a8b95d4499013234a917/server/src/main/java/com/cloud/storage/StorageManagerImpl.java#L619
   > > 
   > > 
   > > I am sure it's not null, not in my case anyway. If you are actually 
referring to the null verification I do in PR then I guess it's just a habit I 
picked up along the way. As you can read in my stacktrace, the hypervisorType 
is not null but rather not castable to string. I am assuming it was never 
accepted in java to cast an enum to string. Enums (at least in C#) are castable 
to integers not strings. If my assumption is wrong then is this code not meant 
to be ran with openjdk?
   > 
   > @alexandru-bagu, it is a general accepted practice to have enum types be 
casted to string, but you'll need a `toString()` defined on the enum. casting 
to int is generally frowned upon because the string descriptors may be 
auto-numbered and an insert on the type will change the numbering. (in this 
case the enum range is small and also persisted so that can't be allowed 
anyway) hope this explains.
   
   My comment was not about whether it's acceptable or not to cast enums to 
integers. It was merely about the fact that C# and Java compilers allow by 
default enum to int casts and vice versa but not enum to string or string to 
enum directly. That comment was obviously related to the fact that in the 
current master branch there is a cast of an Object to String but it turns out 
that Object is sometimes either HypervisorType or String. I opened this PR 
because casting an enum to string (not using helpers/built in functions of 
Enums) causes an exception.
   
   The issue was introduced by 
https://github.com/apache/cloudstack/commit/6df819028e472dd9801873857fe20133381ca0fe
 and I guess no one had this issue because no tests were done using local 
storage.
   
   And as a side note, it seems is is "acceptable"/"ok" to have known blocking 
errors in the upcoming release (4.15.0.0). I mean why not postpone the fix 
until 4.15.1.0? In the meantime anyone who wants to use 4.15.0.0 with local 
storage will just have to patch and build the software themselves I guess. I 
know I will have to.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to