-
Notifications
You must be signed in to change notification settings - Fork 213
Print an error if a *_path
configuration is not a String
#300
Conversation
Mat, could you take a look on this one, too? Other option would be to use
|
@@ -227,6 +227,8 @@ def upload_to_provision_path(src, dest, key_name = 'path') | |||
|
|||
if src.nil? | |||
Chef::Log.debug "'#{key_name}' not set" | |||
elsif !src.respond_to?(:to_str) |
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.
I would have expected :to_s
here (or maybe .is_a? String
). What am I missing?
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.
As #to_s
is defined already in Object
, basically all object have it. So it would lead warnings about missing path (see my previous comment).
Googling lead me to use :to_str
, which means that the object supports implicit conversion. But as it seems that String
is the only Ruby core class that implements is, I agree that .is_a? String
is less confusing. I'll make the change.
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.
Pushed the fix.
Print an error if a `*_path` configuration is not a String
Chef 11.8.0 (with mixlib-config 2.0) sets `*_path` options by default based on `cookbook_path`. If it is an array (as always in knife-solo), the other paths are also arrays. And matschaffer#300 will catch that and error out. While the conversion to Array is a regression in Chef itself, we anyway want to set the default path relative to our kitchen root, as `cookbook_path` components might be pointing to somewhere else.
`node_path` made it back to Chef's default configuration in 11.8.0. And as pre 0.3.0 setups won't normally have it explicitly configured, it defaults to being an Array. Also a user might set it to be an Array, which leads knife-solo crashing with the infamous `ERROR: TypeError: no implicit conversion of Array into String`. This escaped from us in matschafferGH-300.
Fixes #278