Implement asset details collector #1
Conversation
gberginc
changed the title
|
Dependent PR merged, kicking tests |
| private | ||
|
|
||
| def get_server_location(server) | ||
| chassis = [server.Links.Chassis[0]] |
There was a problem hiding this comment.
Is it possible for server.Links.Chassis to be nil or an empty array? If so you might want something like chassis = Array(server.Links.Chassis&.first)
There was a problem hiding this comment.
server.Links.Chassis cannot be nil and it should have only one element in properly configured services. That being said, assuming properly configured services might be a bit naive.
I will rework this part of the code to take into account those edge cases.
| collector.physical_server_details.each do |d| | ||
| server = persister.physical_servers.find_or_build(d[:server_id]) | ||
| details = persister.physical_server_details.find_or_build(server) | ||
| details.assign_attributes( |
There was a problem hiding this comment.
Is it possible for the server details to be built anywhere else? If these can't be created from multiple code paths it'd be better to skip the find_or_build and just use build.
There was a problem hiding this comment.
Server details are only built here. I used find_or_build here because I though that calling build might introduce duplicated entries on inventory refresh. Of course, I may be completely wrong here.
There was a problem hiding this comment.
Ah okay I see what you mean, no the find_or_build part is only relative to the current data in the inventory collection, not what is in the database...that is evaluated during save time.
What you can also do is instead of server = find_or_build(..) you can do a server = persister.physical_servers.lazy_find(d[:server_id]) which will allow the server link to be evaluated at save time even if the server isn't in your current inventory_collections aka you were doing a targeted refresh.
There was a problem hiding this comment.
I updated the code with your recommendations (and made a mental note to read the inventory-related source code once again).
tadeboro
force-pushed
the
redfish-system-assets-collection
branch
from
bf3b61a
to
7a7f8a6
Compare
|
@miq-bot remove_label wip |
miq-bot
changed the title
tadeboro
force-pushed
the
redfish-system-assets-collection
branch
from
7a7f8a6
to
77814f4
Compare
| collector.physical_server_details.each do |d| | ||
| server = persister.physical_servers.lazy_find(d[:server_id]) | ||
| details = persister.physical_server_details.build(server) | ||
| details.assign_attributes( |
There was a problem hiding this comment.
Actually you don't need to assign_attribtues when using build, just pass this hash into build and pass in the manager ref.
So I think for this it'd be:
persister.physical_server_details.build(
:resource => server,
:contact => "",
:description => "",
...
)
There was a problem hiding this comment.
Sorry for the noise, I pushed the partial edit. Things should look better now.
tadeboro
force-pushed
the
redfish-system-assets-collection
branch
from
77814f4
to
fb1142b
Compare
New functionality from this commit stores information that is not strictly related to the physical server to asset_details table that serves as a source of data for summary view in ManageIQ UI. Please take note that asset details mapping is fairly raw right now, since mapping tree-like Redfish structures into flat array of XClarity properties is not trivial and will probably need some tweaking based on real data that we will get from test bed before we get it right.
tadeboro
force-pushed
the
redfish-system-assets-collection
branch
from
fb1142b
to
146e89e
Compare
|
Checked commit xlab-si@146e89e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 **
|
Implement asset details collector
New functionality from this commit stores information that is not
strictly related to the physical server to asset_details table that
serves as a source of data for summary view in ManageIQ UI.
Please take note that asset details mapping is fairly raw right now,
since mapping tree-like Redfish structures into flat array of XClarity
properties is not trivial and will probably need some tweaking based
on real data that we will get from test bed before we get it right.
This pull request needs ManageIQ/manageiq#17486 merged into core.
/cc @gberginc @matejart
@miq-bot assign @gtanzillo
@miq-bot add_label wip