Skip to content
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

Don't queue metrics capture if metrics unsupported #17820

Conversation

agrare
Copy link
Member

@agrare agrare commented Aug 8, 2018

If metrics capture is unsupported by the provider then do not queue
perf_capture for targets on that EMS.

https://bugzilla.redhat.com/show_bug.cgi?id=1610449

@agrare agrare force-pushed the bz_1610449_dont_queue_capture_targets_if_ems_doesnt_support_metrics branch from 3667f73 to 26c52c1 Compare August 8, 2018 14:56
@miq-bot miq-bot added the wip label Aug 8, 2018
@agrare agrare force-pushed the bz_1610449_dont_queue_capture_targets_if_ems_doesnt_support_metrics branch from 26c52c1 to 377d868 Compare August 8, 2018 18:22
@@ -292,6 +292,10 @@ def supports_authentication?(authtype)
authtype.to_s == "default"
end

def supports_metrics?
Copy link
Member Author

@agrare agrare Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO this should be a supports_feature_mixin but kubernetes already has this method defined here

@Axelay-Prime
Copy link

I applied the PR to 5.9.0.22 (appliance) and it corrected the problem.
Upgraded to 5.9.3.4 and re-applied the PR, and it still works.

This looks like a clean and quick fix. I'm chuffed to bits over how elegant that fix is, with just a single line. Thanks for the PR, Adam.

@agrare agrare added the bug label Aug 8, 2018
@agrare agrare changed the title [WIP] Don't queue metrics capture if metrics unsupported Don't queue metrics capture if metrics unsupported Aug 8, 2018
@agrare agrare removed the wip label Aug 8, 2018
If metrics capture is unsupported by the provider then do not queue
perf_capture for targets on that EMS.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1610449
@agrare agrare force-pushed the bz_1610449_dont_queue_capture_targets_if_ems_doesnt_support_metrics branch from 377d868 to d78c076 Compare August 8, 2018 20:25
@miq-bot
Copy link
Member

miq-bot commented Aug 8, 2018

Checked commit agrare@d78c076 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@bdunne bdunne merged commit 6439a45 into ManageIQ:master Aug 9, 2018
@bdunne bdunne assigned bdunne and unassigned blomquisg Aug 9, 2018
@bdunne bdunne added this to the Sprint 92 Ending Aug 13, 2018 milestone Aug 9, 2018
@agrare agrare deleted the bz_1610449_dont_queue_capture_targets_if_ems_doesnt_support_metrics branch August 9, 2018 14:59
simaishi pushed a commit that referenced this pull request Sep 11, 2018
…argets_if_ems_doesnt_support_metrics

Don't queue metrics capture if metrics unsupported
(cherry picked from commit 6439a45)

https://bugzilla.redhat.com/show_bug.cgi?id=1618805
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 3970140bafa2411db17e5d4ef31845aafb58beba
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Thu Aug 9 10:56:49 2018 -0400

    Merge pull request #17820 from agrare/bz_1610449_dont_queue_capture_targets_if_ems_doesnt_support_metrics
    
    Don't queue metrics capture if metrics unsupported
    (cherry picked from commit 6439a450f1818cad5a3282d5d318531bcb41d6a0)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1618805

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.

6 participants