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

Fixes #23233 - updated yum_max_speed example to reflect to_bytes parsing #315

Merged
merged 1 commit into from Apr 13, 2018
Merged

Fixes #23233 - updated yum_max_speed example to reflect to_bytes parsing #315

merged 1 commit into from Apr 13, 2018

Conversation

chris1984
Copy link
Member

@chris1984 chris1984 commented Apr 11, 2018

Updated the init.pp description too since passing Kb fails with an error, as its the to bytes conversion is not happy with uppercase.

@ekohl
Copy link
Member

ekohl commented Apr 11, 2018

If pulp understands these strings, why do we even convert them to bytes in the first place? I'd be happy with removing to_bytes and enforcing a regex that Pulp understands so we never accept invalid input to begin with.

@chris1984
Copy link
Member Author

@ekohl Pulp only understands bytes it was @sean797 who wanted to add the converter in. The converter is the thing that bombs out with Kb instead of kb... do you want to keep it in still since pulp only understands bytes

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I think we should submit a patch to stdlib to make the prefix case insensitive. As a short term fix the documentation should be improved here to mention the odd quirk.

@@ -20,7 +20,7 @@
"proxy_password": null,
<% end %>
<% unless [nil, :undefined, :undef, ''].include?(scope['pulp::real_yum_max_speed']) -%>
"max_speed": <%= scope['pulp::real_yum_max_speed'] %>,
"max_speed": "<%= scope['pulp::real_yum_max_speed'] %>",
Copy link
Member

Choose a reason for hiding this comment

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

If pulp only understands bytes, this change should not be needed.

@@ -186,7 +186,7 @@
#
# $proxy_password:: Proxy password for authentication
#
# $yum_max_speed:: The maximum download speed for a Pulp task, such as a sync. (e.g. "4 Kb" or 1Gb")
# $yum_max_speed:: The maximum download speed for a Pulp task, such as a sync. (e.g. "4 kb" or 1gb")
Copy link
Member

Choose a reason for hiding this comment

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

to_bytes is weird. k[bB] is used, all the others use upper case prefixes: M[Bb] G[Bb] etc.

@chris1984 chris1984 changed the title Fixes #23233 - add quotes to yum speed var Fixes #23233 - update init.pp to reflect to_bytes parsing Apr 13, 2018
@chris1984
Copy link
Member Author

@ekohl updated

@chris1984 chris1984 changed the title Fixes #23233 - update init.pp to reflect to_bytes parsing Fixes #23233 - update yum_max_speed example to reflect to_bytes parsing Apr 13, 2018
@chris1984 chris1984 changed the title Fixes #23233 - update yum_max_speed example to reflect to_bytes parsing Fixes #23233 - updated yum_max_speed example to reflect to_bytes parsing Apr 13, 2018
@ekohl ekohl merged commit 045dedf into theforeman:master Apr 13, 2018
@ekohl
Copy link
Member

ekohl commented Apr 13, 2018

Thanks!

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

3 participants