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
Refs #36219 - allow loading HashWithIndifferentAccess from YAML #9782
Conversation
Issues: #36219 |
@@ -62,7 +62,7 @@ def self.inherited(child) | |||
# extracts serialized metrics and keep them as a hash_with_indifferent_access | |||
def metrics | |||
return {} if self[:metrics].nil? | |||
YAML.safe_load(read_metrics).with_indifferent_access | |||
YAML.safe_load(read_metrics, permitted_classes: [ActiveSupport::HashWithIndifferentAccess]).with_indifferent_access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The alternative solution would be to make read_metrics
not return HWIA, but I wasn't sure the mangling in that function is the only way for the data to get HWIA header, so this seemed safer.
A dumb question: since these errors are hard to catch before they come, isn't it better to define permitted classes as a constant and use it everywhere we use |
I guess? I was first aiming at fixing the nightly issue (which it does, even if I don't understand why the existing tests didn't catch it before). |
[test katello] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A dumb question: since these errors are hard to catch before they come, isn't it better to define permitted classes as a constant and use it everywhere we use
safe_load
?.. Something like https://github.com/ofedoren/foreman/blob/develop/config/application.rb#L181
I think in general you want to limit it as much as possible.
Not sure if this Katello failure is related. I wouldn't think it is.
|
almost sure it isn't [test katello] |
Doesn't look like it, merging. |
Fixes: c9b82a5