Add MiddlewareServerEap and MiddlewareServerWildfly for Utilization charts #2976
Conversation
|
Review @ManageIQ/committers-hawkular |
|
@miq-bot add_label gaprindashvili/yes |
|
@miq-bot add_label bug, middleware |
|
@cfcosta please review |
|
@tumido please review |
|
commit message has a typo (Utilziation) , please fix |
| @@ -634,7 +634,7 @@ def perf_gen_data_before_wait | |||
| from_dt, | |||
| to_dt, | |||
| interval_type] | |||
| elsif %w(MiddlewareServer MiddlewareDatasource MiddlewareMessaging).include?(@perf_record.class.name.demodulize) | |||
| elsif %w(MiddlewareServer MiddlewareServerEap MiddlewareDatasource MiddlewareMessaging).include?(@perf_record.class.name.demodulize) | |||
There was a problem hiding this comment.
what about MiddlewareServerWildfly? it sounds like it would have the same problem, no?
There was a problem hiding this comment.
Yes We'll have the same problem. Added MiddlewareServerWildfly.
aljesusg
force-pushed
the
fix_utilization_EAP_server
branch
2 times, most recently
from
ac79425 to
c8c924d
Compare
|
Edited commit message and added MiddlewareServerWildfly |
aljesusg
changed the title
| @@ -634,7 +634,7 @@ def perf_gen_data_before_wait | |||
| from_dt, | |||
| to_dt, | |||
| interval_type] | |||
| elsif %w(MiddlewareServer MiddlewareDatasource MiddlewareMessaging).include?(@perf_record.class.name.demodulize) | |||
| elsif %w(MiddlewareServer MiddlewareServerEap MiddlewareServerWildfly MiddlewareDatasource MiddlewareMessaging).include?(@perf_record.class.name.demodulize) | |||
There was a problem hiding this comment.
We don't want to add every class every time we decide something should be sub-classed. I would prefer this much more: %w(MiddlewareServer MiddlewareDatasource MiddlewareMessaging).any? { |e| @perf_record.kind_of? e.constantize }
There was a problem hiding this comment.
The problem is that this objct is MiddlewareServerEAP so ..or we control that object include MiddlewareServer or we must check the type.
There was a problem hiding this comment.
Yes I forgot the real-time I'll do after this discussion about how we are going to do it.
There was a problem hiding this comment.
well it works for me. MiddlewareServerEap is still a kind_of? MiddlewareServer so we can check the type properly:
The code I've posted works 😉
There was a problem hiding this comment.
I didn't see that your proposal was kind_of ^^ ok I agree with you, I am gonna make the change Thanks @tumido
aljesusg
force-pushed
the
fix_utilization_EAP_server
branch
from
c8c924d to
feb3f65
Compare
|
I did the changes @tumido thanks |
aljesusg
force-pushed
the
fix_utilization_EAP_server
branch
from
feb3f65 to
f7e71ea
Compare
…ap and MiddlewareServerWildfly for Utilization charts
aljesusg
force-pushed
the
fix_utilization_EAP_server
branch
from
f7e71ea to
0ec98f3
Compare
|
Checked commit aljesusg@0ec98f3 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
This looks much better! 🎉
I know the test coverage is poor in this area (and refactoring is needed), so I leave that on others to decide - can we have this tiny part covered by tests? Something like "with these entities we can at least render a chart" testcases.
Add MiddlewareServerEap and MiddlewareServerWildfly for Utilization charts (cherry picked from commit bec1a6f) https://bugzilla.redhat.com/show_bug.cgi?id=1524706
|
Gaprindashvili backport details: |
Bugzilla #1510548
MiddlewareServerEap and MiddlewareServerWildfly was not in the list of types.
cc @abonas