pipenv icon indicating copy to clipboard operation
pipenv copied to clipboard

excessive logs despite "--quiet" when running "pipenv check"

Open MalteMagnussen opened this issue 1 year ago • 2 comments

Issue description

Running pipenv --bare --quiet check --quiet --use-installed still produces lots of INFO logs.

Expected result

I'd like to only see the following:

(using image because of error There was an error creating your Issue: body is too long (maximum is 65536 characters). )

image

Actual result

I am seeing all of these "INFO" and "LINE" logs that I am not interested in, when using --quiet

image

Steps to replicate

Run pipenv --bare --quiet check --quiet --use-installed and see the excessive output, despite --quiet

Uploading the output of pipenv --support because of "body too long" error.

support.txt

MalteMagnussen avatar Aug 30 '24 12:08 MalteMagnussen

We bundle safety in pipenv but we have never modified it to change its logging behaviors. Doing so would require a .patch file that modified the vendored dependency, which may be more trouble than its worth IMO for the safety sub-command.

matteius avatar Sep 28 '24 23:09 matteius

Analysis of Pipenv Issue #6228

1. Problem Summary:

The pipenv check command produces excessive log output even when the --quiet flag is used. The user expects only a concise summary of the vulnerabilities found and the overall status, similar to the output of Safety's --bare option.

2. Comment Analysis:

  • The maintainer correctly points out that Safety, a vendored dependency of Pipenv, controls the logging behavior.
  • Patching Safety to change its logging would require maintaining a patch file, potentially increasing maintenance overhead.

3. Proposed Resolution:

Instead of patching Safety, we can modify Pipenv's do_check function in pipenv/routines/check.py to handle the logging output more effectively. The proposed solution involves the following:

  • Capture Output: Capture the stdout/stderr output from the safety check subprocess.
  • Conditional Logging: Based on the value of quiet flag: - If quiet=True, parse the JSON output from safety check and only display a concise summary, similar to Safety's --bare output. - If quiet=False, display the captured output as usual.

4. Code Snippet (pipenv/routines/check.py):

def do_check(
	  # ... existing arguments
	  quiet=False,
	  # ... existing arguments
):
	  # ... (Existing Code) ...

	  # Run the safety check command
	  c = run_command(cmd, is_verbose=project.s.is_verbose(), capture_output=True)

	  if c.returncode != 0:
	      # ... existing error handling

	  # Handle output based on quiet flag
	  if quiet:
	      try:
	          report = json.loads(c.stdout.decode())
	          vulnerabilities_found = report.get("report_meta", {}).get("vulnerabilities_found", 0)
	          click.secho(f"{vulnerabilities_found} vulnerabilities found.", fg="red" if vulnerabilities_found else "green")
	          # ... potentially add a concise summary based on report data
	      except json.JSONDecodeError:
	          raise exceptions.PipenvCmdError(cmd, c.stdout, c.stderr, c.returncode)
	  else:
	      # ... (Existing output handling) ...

5. Additional Steps:

  • Logging Configuration: Currently, the log level for the "pipenv" logger is set to WARN if verbose=False. This should be changed to a higher level (e.g., ERROR) to suppress unnecessary INFO logs when quiet=True.
  • Refactor run_command: The run_command function could be enhanced to support capturing output by default, simplifying the code in do_check.
  • Testing: Add tests to verify the behavior of pipenv check with the --quiet flag, ensuring that output is minimal and concise.

This solution addresses the issue of excessive logging while retaining the existing functionality of safety check and avoiding direct modifications to the vendored dependency. This approach enhances the user experience by providing cleaner output when using the --quiet flag.

matteius avatar Oct 18 '24 21:10 matteius

@oz123 <3 thank you very much

MalteMagnussen avatar Oct 22 '24 18:10 MalteMagnussen