Skip to content
This repository has been archived by the owner on Mar 19, 2022. It is now read-only.

Print an error if a *_path configuration is not a String #300

Merged
merged 1 commit into from
Oct 19, 2013

Conversation

tmatilai
Copy link
Collaborator

Fixes #278

@tmatilai
Copy link
Collaborator Author

Mat, could you take a look on this one, too?

Other option would be to use src.to_s in upload_to_provision_path, but for arrays the error message would be still quite cryptic:

WARN: Local role_path '["foo", "bar"]' does not exist

@@ -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)
Copy link
Owner

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pushed the fix.

matschaffer added a commit that referenced this pull request Oct 19, 2013
Print an error if a `*_path` configuration is not a String
@matschaffer matschaffer merged commit 382d017 into matschaffer:master Oct 19, 2013
@tmatilai tmatilai deleted the non-string-paths branch November 8, 2013 19:25
tmatilai added a commit to tmatilai/knife-solo that referenced this pull request Nov 8, 2013
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.
tmatilai added a commit to tmatilai/knife-solo that referenced this pull request Nov 8, 2013
`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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERROR: TypeError: can't convert Array into String
2 participants