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.

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

In get_conda_pkg_info: do not have subprocess.run check the return code. #111

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

javierggt
Copy link
Collaborator

@javierggt javierggt commented Apr 18, 2024

This fixes some unwanted output, and fixes packages.get_conda_pkg_info so it works for github repositories that do not have a corresponding package with the same name.

The code was searching for a conda package named the same way as the repo, which is trivially not true in repos that have no package at all.

This is a trivial change. Basically, the check argument passed to subprocess.run was popped from a kwargs variable that was local and never had the check key, so it was always being set to True. Also, check=True was obviously not the original intent of this code, since right after the call to subprocess.run the output was being parsed and inspected for errors, optionally ignoring them in exactly the cases we care about.

This affects nothing we care about. After the PR, there will be package info for these repos, but we do not care about them.

Description

Fixes #

Interface impacts

Testing

Unit tests

  • No unit tests

Functional tests

Ran the update dashboard script and there was no unwanted output.

…er the call, and it is ignored if the package is not found.
@javierggt javierggt requested a review from jeanconn April 18, 2024 15:32
@chandra-xray chandra-xray merged commit 3fb5da9 into master Apr 29, 2024
2 checks passed
@javierggt javierggt deleted the non-package-repo branch July 16, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants