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.

Sign up for GitHub

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

Jump to bottom

Fix nodejs_helper to use proper key in npm json #40

Closed
wants to merge 1 commit into from
Closed

Fix nodejs_helper to use proper key in npm json #40

wants to merge 1 commit into from

Conversation

petecheslock
Copy link

This Fixes #39

In https://github.com/redguide/nodejs/blob/master/libraries/nodejs_helper.rb#L33-L37 section of the library - it looks like it's using the wrong key to do the proper check for the package being installed.

For example, when debugging in chef-shell:

list = npm_list('/opt/my-app')
chef > pp list
{"name"=>"my-app",
 "version"=>"0.1.15",
 "dependencies"=>
  {"async"=>
    {"version"=>"0.2.10",
     "from"=>"async@~0.2.9",
     "resolved"=>"https://registry.npmjs.org/async/-/async-0.2.10.tgz"},
<ship>

chef > package = 'my-app'
 => "my-app"
chef > list.key?(package)
 => false
chef > list[package]['version']
NoMethodError: undefined method `[]' for nil:NilClass
        from (irb):22
        from /opt/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.14.6/lib/chef/shell.rb:75:in `block in start'
        from /opt/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.14.6/lib/chef/shell.rb:74:in `catch'
        from /opt/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.14.6/lib/chef/shell.rb:74:in `start'
        from /opt/chef/embedded/lib/ruby/gems/1.9.1/gems/chef-11.14.6/bin/chef-shell:37:in `<top (required)>'
        from /usr/bin/chef-shell:23:in `load'
        from /usr/bin/chef-shell:23:in `<main>'

But if I make the changes like in this PR - this is what i see

chef > list = npm_list('/opt/my-app')
chef > list['name']
 => "my-app"
chef > list['version']
 => "0.1.15"

@guilhem
Copy link
Contributor

guilhem commented Sep 18, 2014

Hi @petecheslock
Thanks for this patch. But it only work with path mode of npm.
As you can see with npm list -global -json your command will fail.

@guilhem
Copy link
Contributor

guilhem commented Sep 18, 2014

Morover in my case, in path mode, my list look like that

@guilhem guilhem mentioned this pull request Sep 18, 2014
Open
@petecheslock
Copy link
Author

Later last night after I sent the PR I wondered about the non json installs. Looks like that is going to be a bit trickier. Those examples do help though I might take a closer look and see. Or just wrap the resource with a conditional or something.

On Sep 18, 2014, at 4:33 AM, Guilhem Lettron notifications@github.com wrote:

Morover in my case, in path mode, my list look like that

—
Reply to this email directly or view it on GitHub.

@BarthV
Copy link
Contributor

BarthV commented Jan 29, 2016

Closing this one. Please repoen if needed.

@BarthV BarthV closed this Jan 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

nodejs_npm with json is not idempotent
3 participants
@petecheslock @guilhem @BarthV