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

set regular logging level to ERROR #5957

Merged

Conversation

fmssn
Copy link
Contributor

@fmssn fmssn commented Sep 30, 2023

The issue

Fix noisy output in pipenv check as described in issue #5939. Prior to release v2023.8.19 in pipenv/utils/__init__.py logging basic config was set to ERROR, then to INFO. I am not sure whether that was on purpose, was changed in a large commit #5793, maybe it was just used for debugging and then not changed back. @matteius

The fix

Change back to logging.ERROR.

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

@matteius
Copy link
Member

matteius commented Sep 30, 2023

The change to INFO was intentional -- quiet/bare should be fixed similar to how it was addressed here:
#5897

Otherwise we will not be able to debug resolution internal to pip -- there were plenty of other issue reports related, which is why I made the change you referenced initially. My stance is yes we want to better support quiet/bare, but its better to have too much information than not enough, and so I would rather leave the default as INFO and override specific to the quiet/bare flags.

- add verbosity arg to check command, if verbosity if None (default),
    logging level is set to warn
- create a build_options function in check.py for handling all arguments
    used to build the options string for pipenv check. Was needed
    because otherwise there were too many branches for the linter.
@fmssn
Copy link
Contributor Author

fmssn commented Oct 2, 2023

@matteius I adjusted the code fitting to your recent pull request, setting to warn if not verbose.

I would like to have some thought based on how to pass the verbosity arg to the check function in pipenv/cli/command.py: There is the possibility passing it via 'direct' argument, inserting verbose=False into the args, and the possibility of getting it via the State object. Both worked in my tests. I decided to go for the latter choice, however I am not 100% sure on that, as for example the quiet argument could, afaik, also be passed via the State, yet this is not the case. I would be grateful for further feedback.

@matteius
Copy link
Member

matteius commented Oct 3, 2023

I think the click args would all get passed via the state, the way you are doing it -- this change looks reasonable to me.

@matteius matteius merged commit a725b20 into pypa:main Oct 3, 2023
16 checks passed
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