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

[HAMMER] - Fixed display of worker memory threshold settings #5323

Merged
merged 1 commit into from Mar 14, 2019

Conversation

h-kataria
Copy link
Contributor

@h-kataria h-kataria commented Mar 12, 2019

Fixed couple of scenarios

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1686901
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1686902

(cherry picked from commit 6a20b57)

backport of #5189 @hstastna @himdel if one you can veirfy this fix on hammer, that will be great, i am having issues running hammer branch locally.

@hstastna
Copy link
Contributor

I was able to setup Hammer and to test this PR and here is what I've found:

For https://bugzilla.redhat.com/show_bug.cgi?id=1656873,
I can see no change, it does not work well for Event Monitor, Refresh and Connection Broker, after saving the changes it shows the smallest values from the drop downs. Here is the screenshot after setting all the possible tresholds to 1.1 GB:
tresholds

For https://bugzilla.redhat.com/show_bug.cgi?id=1658373,
it seems to work (tried more different values), the yaml file seems to change (Advanced tab) after changing the values in the UI, under Workers tab. Here is the screenshot after changing all the four values of tresholds (for Generic Workers, Priority Workers, C & U Data Collectors and C & U Data Processors) to 450 MB:
defaults

Fixed couple of scenarios
- Updated code to use `get_worker_setting` method
- Worker memory threshold of 1 GB or above was not reflected on Workers tab because https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/controllers/ops_controller/settings/common.rb#L997 value of variable `x` needs to be converted to integer value from a float so value can be selected correctly in the drop down.
- If a value in yaml config file is set to as 600.megabytes, it is being returned as a string, so need to evaluate/convert the value so it can be correctly selected in the drop down.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1656873
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1658373

(cherry picked from commit 6a20b57)
@miq-bot
Copy link
Member

miq-bot commented Mar 13, 2019

Checked commit h-kataria@142b265 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. ⭐

Copy link
Contributor

@hstastna hstastna left a comment

Choose a reason for hiding this comment

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

After some additional changes, it works well now. I approve this.

@simaishi simaishi merged commit e6b9ef9 into ManageIQ:hammer Mar 14, 2019
@simaishi simaishi added this to the Sprint 107 Ending Mar 18, 2019 milestone Mar 14, 2019
@h-kataria h-kataria deleted the pr_5189 branch April 9, 2019 22:04
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.

None yet

4 participants