Skip to content

Conversation

@zizhongwei
Copy link
Contributor

Fix cpu info in hypervisor for #1012

@auhlig
Copy link
Member

auhlig commented May 24, 2017

Hi @zizhongwei,
Thanks for the PR.
Could you wrap some test around that? Why are you using List<Object>?

@auhlig auhlig added this to the 3.1.0 Release milestone May 24, 2017
@zizhongwei
Copy link
Contributor Author

zizhongwei commented May 25, 2017

there is a note in the openstack api description:
Since version 2.28 cpu_info field is returned as a dictionary instead of string.
When I use the Hypervisor list api to get the hypervisors,it returned a JSON like this:
"cpu_info": {"model": ["Intel(R)Xeon(R)CPUE5-2620v2@2.10GHz"], "vendor": ["GenuineIntel"], "nodes": 1, "topology": {"cores": 12, "threads": 24}}
in this JSON, "model" field is ["Intel(R)Xeon(R)CPUE5-2620v2@2.10GHz"] and "vendor" field is ["GenuineIntel"].
In my test the Jackson can reads this "model" field to List<Object>(can not reads this "model" field to String)
and the Jackson can also reads JSON "model": "Intel(R)Xeon(R)CPUE5-2620v2@2.10GHz" to List<Object>
so I replaced String with List<Object>

@auhlig
Copy link
Member

auhlig commented May 30, 2017

Wouldn't​ List<Object> be incompatible with the json-string, whereas the type string would work for both?

@zizhongwei
Copy link
Contributor Author

zizhongwei commented May 31, 2017

openstack4j has used Jackson read json to java Object.
Jackson can read json-string :"model": "Intel(R)Xeon(R)CPUE5-2620v2@2.10GHz" to List<Object>, also can read json :"model": ["Intel(R)Xeon(R)CPUE5-2620v2@2.10GHz"] to List<Object>;
Jackson can not read json :"model": ["Intel(R)Xeon(R)CPUE5-2620v2@2.10GHz"] to String ,will throw a exception like : #1012

@auhlig
Copy link
Member

auhlig commented May 31, 2017

I see. LGTM. Thanks @zizhongwei

@auhlig auhlig added the bug label May 31, 2017
@auhlig auhlig merged commit e90d236 into ContainX:master May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants