-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding summary for number of resources, racks and health states to Provider #17841
Adding summary for number of resources, racks and health states to Provider #17841
Conversation
8de93d2
to
5a67ca2
Compare
virtual_column :total_valid, :type => :integer | ||
virtual_column :total_warning, :type => :integer | ||
virtual_column :total_critical, :type => :integer | ||
virtual_column :health_state_info, :type => :string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are you planning on using this virtual column? It's defined as :string
but the method returns a hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gtanzillo The intention is to add it to the physical infra provider JSON response at the ManageIQ API. I put it as a :string
because there is no type :hash
for virtual columns. What I could do is do create these columns as :json
, I didn't do it before because I didn't know it was possible. What do you think of that? It's more accurate, and fit to the purpose, then :string
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EsdrasVP Yes, I think that the :json
type makes more sense here and I would go with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gtanzillo Great, I made the change, so :json
will be used instead of :string
.
5a67ca2
to
5ed1437
Compare
Checked commit EsdrasVP@5ed1437 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 app/models/manageiq/providers/physical_infra_manager.rb
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
This PR is able to:
This should be displayed at the API by accessing
/api/providers/:id?attributes=resources_info
/api/providers/:id?attributes=health_state_info
If both are accessed, the response is presented as follows: