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

TemporaryPipInstallResolver: Prevent pip install from polluting stdout #361

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

jherland
Copy link
Member

I noticed this while playing around with --install-deps and --json, and
at the same time piping the JSON output into jq for filtering/colors:
The pip install that is triggered by --install-deps will print status
messages to both stdout and stderr, and since these are not handled by
our subprocess.run() call, they will appear directly on our
stdout/stderr. When piping our stdout to jq this causes jq to get
those status messages (preceding the JSON output) on its stdin, and it
predictably aborts with an error message.

This commit fixes the issue in two ways:

First, we pass --quiet and --disable-pip-version-check to pip install,
to silence some common/unnecesary output. This seems sufficient to make
pip install silent when everything works as expected. However, when
warnings and errors might still be printed in exceptional cases.

Next, we capture the stdout + stderr from pip install and only print
them in case the pip install command failed.

With both of these fixes, we ensure that pip install only makes noise
when it fails, and that noise only appears on our stderr (in the shape
of a warning log message). For example, if I try to --install-deps with
an intentional typo (pyyamll):

  WARNING:fawltydeps.packages:Command failed: ['/run/user/1000/tmpr5l6m94i/bin/pip', 'install', '--no-deps', '--quiet', '--disable-pip-version-check', 'pyyamll', 'scikit-learn']
  WARNING:fawltydeps.packages:Output:
  ERROR: Could not find a version that satisfies the requirement pyyamll (from versions: none)
  ERROR: No matching distribution found for pyyamll

  WARNING:fawltydeps.packages:Failed to install 'pyyamll'
  WARNING:fawltydeps.packages:Output:
  ERROR: Could not find a version that satisfies the requirement pyyamll (from versions: none)
  ERROR: No matching distribution found for pyyamll

  ERROR:fawltydeps.main:Unresolved dependencies: pyyamll
  FawltyDeps is unable to find the above packages with the configured package resolvers. Consider using --pyenv if these packages are already installed somewhere, or --custom-mapping-file to take full control of the package-to-import-names mapping.

I noticed this while playing around with --install-deps and --json, and
at the same time piping the JSON output into `jq` for filtering/colors:
The `pip install` that is triggered by --install-deps will print status
messages to both stdout and stderr, and since these are not handled by
our subprocess.run() call, they will appear directly on _our_
stdout/stderr. When piping our stdout to `jq` this causes `jq` to get
those status messages (preceding the JSON output) on its stdin, and it
predictably aborts with an error message.

This commit fixes the issue in two ways:

First, we pass --quiet and --disable-pip-version-check to `pip install`,
to silence some common/unnecesary output. This seems sufficient to make
`pip install` silent when everything works as expected. However, when
warnings and errors might still be printed in exceptional cases.

Next, we capture the stdout + stderr from `pip install` and only print
them in case the `pip install` command failed.

With both of these fixes, we ensure that `pip install` only makes noise
when it fails, and that noise only appears on our stderr (in the shape
of a warning log message). For example, if I try to --install-deps with
an intentional typo ("pyyamll):

  WARNING:fawltydeps.packages:Command failed: ['/run/user/1000/tmpr5l6m94i/bin/pip', 'install', '--no-deps', '--quiet', '--disable-pip-version-check', 'pyyamll', 'scikit-learn']
  WARNING:fawltydeps.packages:Output:
  ERROR: Could not find a version that satisfies the requirement pyyamll (from versions: none)
  ERROR: No matching distribution found for pyyamll

  WARNING:fawltydeps.packages:Failed to install 'pyyamll'
  WARNING:fawltydeps.packages:Output:
  ERROR: Could not find a version that satisfies the requirement pyyamll (from versions: none)
  ERROR: No matching distribution found for pyyamll

  ERROR:fawltydeps.main:Unresolved dependencies: pyyamll
  FawltyDeps is unable to find the above packages with the configured package resolvers. Consider using --pyenv if these packages are already installed somewhere, or --custom-mapping-file to take full control of the package-to-import-names mapping.
@jherland jherland changed the title TemporaryPipInstallResolver: Prevent pip install from polluting stdout TemporaryPipInstallResolver: Prevent pip install from polluting stdout Aug 29, 2023
fawltydeps/packages.py Show resolved Hide resolved
fawltydeps/packages.py Show resolved Hide resolved
@jherland jherland merged commit 8821172 into main Aug 30, 2023
23 checks passed
@jherland jherland deleted the jherland/silent-temp-pip-install branch August 30, 2023 15:04
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.

2 participants