Skip cached cookbooks missing their name attributes instead of failing #1198
Conversation
| begin | ||
| CachedCookbook.from_store_path(path) | ||
| rescue Ridley::Errors::MissingNameAttribute | ||
| # Skip cached cookbooks that do not have a name attribute. |
There was a problem hiding this comment.
Perhaps print a warning here so we know a cookbook is being skipped.
There was a problem hiding this comment.
I'm thinking:
[WARNING] Skipping '#{name}'. Berkshelf can only interact with cookbooks which
have defined the `name` attribute in the metadata.rb. If you are the maintainer
of '#{name}', please add the name attribute to your cookbook. If you are not the
maintainer of '#{name}', please file an issue or report the lack of a name
attribute as a bug.
You can remove '#{name}' from the Berkshelf shelf by using the `berks shelf uninstall`
command:
$ berkshelf shelf uninstall #{name}
There was a problem hiding this comment.
I think we should also include instructions on how to remove the cookbook from the shelf with the shelf uninstall command
There was a problem hiding this comment.
👍 - updated my sample text
|
👍 |
|
@KAllan357 I think it's fine to skip past the broken ones |
|
I agree with @bluepojo - we should definitely tell the user we skipped it. |
| @@ -102,6 +102,58 @@ def generate_cookbook(path, name, version, options = {}) | |||
| cookbook_path | |||
| end | |||
|
|
|||
| def generate_cookbook_without_name(path, name, version, options = {}) | |||
| path = Pathname.new(path) | |||
There was a problem hiding this comment.
Seems like we could just call generate_cookbook in here and then just remove the first line from the file. It would definitely be simpler and less code to manage.
|
@KAllan357 what's the status on this? Any update/comments you wanna chat about? 😄 |
|
@sethvargo @KAllan357 the comments regarding the warning need to be addressed and then I think we're good to go |
|
I'll be happy to address the comments when I get back on Monday (on vacation right now 😄) |
|
@reset / @sethvargo I believe this last commit should address the comments! |
| end.compact | ||
|
|
||
| if skipped_cookbooks.any? | ||
| msg = "Skipping cookbooks '#{skipped_cookbooks.join(',')}'. Berkshelf can only interact " |
There was a problem hiding this comment.
I think this should be a list at the end rather than a 'sentence'. I think it will be much easier for people to identify the cookbooks as such.
|
@KAllan357 looks great to me man |
|
👍 |
Skip cached cookbooks missing their name attributes instead of failing
For as long as CHEF-3490 is a thing, it is my opinion that berkshelf should not fail when there exists a cached cookbook that is missing its name attribute.
In our scenario, we have written an estimated 75 cookbooks, and migrating them to use Berkshelf 3 will be an ongoing task. If my development environment (or CI server) has a mix of BS 2 and 3 cookbooks, I'd expect to have many cookbooks that are somewhere between "poopy" and "awesome" in terms of quality.
When I go to execute a
berks installin a mixed environment like this, I currently get errors like the following:Regardless of whether or not my cookbook depends on these bad cookbooks, I receive a failure.
At the moment, any cookbooks cache that is tainted by one of these cookbooks will fail every berkshelf 3 run on that machine. You either
rm -rfyour cookbooks directory, delete the bad cookbook from your Chef server, or release a new version of the old cookbook with a shiny new name attribute. Ultimately, you still have torm -rfthe bad cookbook because its still sitting on your disk, and breakingberks install.Another annoying part is that your bad cookbooks fail one-at-a-time. Deleting just one broken cookbook from your disk might not get berkshelf working again - there may be more bad cookbooks that it just never got to parse because of the raised exception.
What do you think? @reset @ivey @sethvargo