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

[MiqWorker::worker_settings] Handle number strings #18453

Merged

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Feb 13, 2019

Due to some UI specific issues, settings in the config can be set as "integer strings", or a string that is just a integer (floats are also possible). This doesn't work booting workers as it initializes them with incorrect values.

This does some extra checking to avoid that.

Links

@jrafanie
Copy link
Member

LGTM. Anything outside of the workers section won't get this fix but hopefully most of the number strings people set are for the workers.

@NickLaMuro NickLaMuro force-pushed the handling_integer_strings_in_worker_settings branch from 8b440fe to 7a4c8d0 Compare February 13, 2019 15:51
app/models/miq_worker.rb Outdated Show resolved Hide resolved
@Fryguy
Copy link
Member

Fryguy commented Feb 13, 2019

Overall LGTM...had one minor comment but we could probably merge without it.

Due to some UI specific issues, settings in the config can be set as
"integer strings", or a string that is just a integer (floats are also
possible).  This doesn't work booting workers as it initializes them
with incorrect values.

This does some extra checking to avoid that.
@NickLaMuro NickLaMuro force-pushed the handling_integer_strings_in_worker_settings branch from 7a4c8d0 to eee01fb Compare February 13, 2019 17:31
@miq-bot
Copy link
Member

miq-bot commented Feb 13, 2019

Checked commit NickLaMuro@eee01fb 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. 🍰

@Fryguy Fryguy merged commit 2f897c5 into ManageIQ:master Feb 13, 2019
@Fryguy Fryguy self-assigned this Feb 13, 2019
@Fryguy Fryguy added the bug label Feb 13, 2019
@Fryguy Fryguy added this to the Sprint 105 Ending Feb 18, 2019 milestone Feb 13, 2019
@NickLaMuro
Copy link
Member Author

@miq-bot add_label hammer/yes

simaishi pushed a commit that referenced this pull request Feb 14, 2019
…_worker_settings

[MiqWorker::worker_settings] Handle number strings

(cherry picked from commit 2f897c5)

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

Hammer backport details:

$ git log -1
commit 4568a21eb8cac0097b508d1c3f345c9005bd913a
Author: Jason Frey <fryguy9@gmail.com>
Date:   Wed Feb 13 13:00:57 2019 -0500

    Merge pull request #18453 from NickLaMuro/handling_integer_strings_in_worker_settings
    
    [MiqWorker::worker_settings] Handle number strings
    
    (cherry picked from commit 2f897c5d50fdb89f4f8615c2f7d27c9b35c2690e)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1677409

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.

5 participants