Pw/pip windows bug #2883
Merged
Pw/pip windows bug #2883
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When checking pip resources, we should skip resource if python is not installed or we will fail with an error when trying to parse the path.
On Windows, if pip has a newer version available, it adds an error message to stderr. Now checking if both stderr and stdout on windows have values. If so, assume pip package is installed. Fixes #2855 Signed-off-by: Paul Welch <[email protected]>
pwelch
force-pushed
the
pw/pip-windows-bug
branch
from
350cd78 to
161ddf3
Compare
jquick
reviewed
Mar 27, 2018
lib/resources/pip.rb
Outdated
| end | ||
|
|
||
| def cmd_successful? | ||
| result = inspec.command("#{@pip_cmd} show #{@package_name}") |
There was a problem hiding this comment.
Could we just use cmd here instead of doing the inspec.command again?
lib/resources/pip.rb
Outdated
| # {"Pip" => nil, "Python" => "/path/to/python"} | ||
| # | ||
| # @return [Hash] of paths | ||
| def paths |
There was a problem hiding this comment.
You mind updating this to windows_paths. Since its windows only command it may read better.
There was a problem hiding this comment.
@jquick good catch! Thanks!
Signed-off-by: Paul Welch <[email protected]>
lib/resources/pip.rb
Outdated
| end | ||
|
|
||
| def info | ||
| return @info if defined?(@info) | ||
|
|
||
| @info = {} | ||
| @info[:type] = 'pip' | ||
| cmd = inspec.command("#{@pip_cmd} show #{@package_name}") | ||
| return @info if cmd.exit_status != 0 | ||
| return @info if !cmd_successful? |
There was a problem hiding this comment.
return @info unless cmd_successful?
lib/resources/pip.rb
Outdated
| end | ||
|
|
||
| def cmd_successful? | ||
| result = cmd |
There was a problem hiding this comment.
Seems like every instance of result in this method could just be cmd? (And would actually read better that way?)
lib/resources/pip.rb
Outdated
| # @return [Hash] of windows_paths | ||
| def windows_paths | ||
| return @__windows_paths if @__windows_paths | ||
| cmd = inspec.command('New-Object -Type PSObject | Add-Member -MemberType NoteProperty -Name Pip -Value (Invoke-Command -ScriptBlock {where.exe pip}) -PassThru | Add-Member -MemberType NoteProperty -Name Python -Value (Invoke-Command -ScriptBlock {where.exe python}) -PassThru | ConvertTo-Json') |
There was a problem hiding this comment.
Can we break this out onto multiple lines?
cmd = inspec.command(
'New-Object -Type PSObject' \
' | Add-Member -MemberType NoteProperty -Name Pip -Value (Invoke-Command -ScriptBlock {where.exe pip}) -PassThru' \
' | Add-Member -MemberType NoteProperty -Name Python -Value (Invoke-Command -ScriptBlock {where.exe python}) -PassThru' \
' | ConvertTo-Json')or better...
cmd = inspec.command([
'New-Object -Type PSObject',
'Add-Member -MemberType NoteProperty -Name Pip -Value (Invoke-Command -ScriptBlock {where.exe pip}) -PassThru',
'Add-Member -MemberType NoteProperty -Name Python -Value (Invoke-Command -ScriptBlock {where.exe python}) -PassThru',
'ConvertTo-Json'
].join(' | '))
|
@TrevorBramble changes for feedback applied. |
- Make it easier to read what the powershell command is doing - Make it easier to read what the cmd_successful method lokos for Signed-off-by: Paul Welch <[email protected]>
pwelch
force-pushed
the
pw/pip-windows-bug
branch
from
653542b to
860c951
Compare
jquick
approved these changes
Mar 29, 2018
TrevorBramble
approved these changes
Mar 29, 2018
|
Confirmed commit was signed. DCO bot is bugged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When pip on windows has a newer version available it spits out a stderr message along with the results of the
pip showcommand.This adds logic to ignore the pip message that there is a newer version.
Fixes #2855