Worker Settings: save memory threshold as an integer #4633
Conversation
Saving the memory threshold value as a string would prevent the value to be read / used correctly later by the worker itself. https://bugzilla.redhat.com/show_bug.cgi?id=1624431
mzazrivec
force-pushed
the
save_worker_memory_threshold_as_integer
branch
from
e89db53 to
e602fdf
Compare
|
Checked commit mzazrivec@e602fdf with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
mzazrivec
changed the title
| @@ -835,43 +835,43 @@ def settings_get_form_vars | |||
|
|
|||
| w = qwb[:generic_worker] | |||
| w[:count] = params[:generic_worker_count].to_i if params[:generic_worker_count] | |||
| w[:memory_threshold] = params[:generic_worker_threshold] if params[:generic_worker_threshold] | |||
| w[:memory_threshold] = params[:generic_worker_threshold].to_i if params[:generic_worker_threshold] | |||
There was a problem hiding this comment.
Shouldn't we be using to_i_with_method here?
Or is every single use of 200.megabytes gone?
Because...
200.megabytes.to_i == 200.megabytes == 209715200
but
"200.megabytes".to_i == 200
whereas
"200.megabytes".to_i_with_method == 200.megabytes.to_i_with_method == 209715200
There was a problem hiding this comment.
From log:
Processing by OpsController#settings_form_field_changed as JS
INFO -- : Parameters: {"generic_worker_threshold"=>"367001600", "id"=>"workers"}
i.e. what you're getting from the browser here is "367001600", not "350.megabytes".
There was a problem hiding this comment.
Aaaah, never mind :)
It looked like the data could come from the db, didn't notice it's always params.
You're right 👍
There was a problem hiding this comment.
Thanks for digging into this. Yes, either "350.megabytes"(string) or 367001600(integer) would be acceptable, but not "367001600"(string). This change looks good to me although I'm not quite sure of how the values are transformed from form to backend.
…as_integer Worker Settings: save memory threshold as an integer (cherry picked from commit a80af62) https://bugzilla.redhat.com/show_bug.cgi?id=1629897
|
Gaprindashvili backport details: |
Saving the memory threshold value as a string would prevent the value to be read / used correctly later by the worker itself.
See https://bugzilla.redhat.com/show_bug.cgi?id=1624431#c6
https://bugzilla.redhat.com/show_bug.cgi?id=1624431