-
Notifications
You must be signed in to change notification settings - Fork 24
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.
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
Fix parsing of conda list output #9
Conversation
For transparency, do we know which versions of conda this will work with? |
I don't think it's the format of I'd recommend using the |
Thanks for the review, I can add the regexp back and test on my deployment.
I don't have any idea of what versions of conda this will work with.
I did some tests of recent conda versions, and they definitely don't return
stdout that matches the expectation of the previous logic. I don't know
which versions of conda that returned output that did work with the logic.
…On Thu, Jan 5, 2017 at 1:06 PM, Matt Davis ***@***.***> wrote:
I don't think it's the format of conda list that has changed, rather that
the return code doesn't relate to whether the package of interest is
installed or not.
I'd recommend using the --json flag but leaving the regex on the name in
there because that still seems to work, e.g. conda list '^certifi$' --json.
As written this PR would falsely mix packages that have the same ending,
consider chunks and django-chunks (both real packages). Using the regex
input to conda list and doing some more strict checking in the function
would protect against those kind of mix ups and I don't think you'd even
need to loop over the results anymore.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#9 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABr0D9pmVkx14f7_VlfFAYRvKndV-Imks5rPU01gaJpZM4Lb-xD>
.
--
Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com
|
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.
One missing comma, otherwise looks good.
@@ -114,7 +115,8 @@ def _check_installed(module, conda, name): | |||
command = [ | |||
conda, | |||
'list', | |||
'^' + name + '$' # use regex to get an exact match | |||
'^' + name + '$' |
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.
Missing a trailing ,
here.
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.
good eye, just push a fix
Thanks Matt!!
…On Thu, Jan 5, 2017 at 4:17 PM, Matt Davis ***@***.***> wrote:
Merged #9 <#9>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#9 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABr0AqcZazEe9okMZcR0GRL403WBIvRks5rPXn0gaJpZM4Lb-xD>
.
--
Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com
|
I'm using conda 4.3.11 and the output is now different, so this patch is not working because it's returning a dict instead of string:
If you want, I can write another patch that handle this. |
The format of conda list has changed, so this ansible module is broken. This fixes it to use
conda list --json
.