Skip to content

Log ending round state @ during resolver debug #12264

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

Conversation

webknjaz
Copy link
Member

@webknjaz webknjaz commented Sep 6, 2023

This change corrects a tiny-little mistake made in 9f318de that was causing the word "state" to be printed out in the log instead of the value of the ending_round()'s state` argument.

@webknjaz
Copy link
Member Author

webknjaz commented Sep 6, 2023

(not sure if this is worthy of a change note)

@webknjaz webknjaz requested a review from pradyunsg September 6, 2023 23:21
@webknjaz webknjaz added type: bug A confirmed bug or unintended behavior C: dependency resolution About choosing which dependencies to install C: cli Command line interface related things (optparse, option grouping etc) C: error messages Improving error messages labels Sep 6, 2023
@pradyunsg
Copy link
Member

This is not a typo -- encoding the entire state into the message was deemed not useful and causes a lot of noise when extremely long resolves take place.

@webknjaz
Copy link
Member Author

webknjaz commented Sep 7, 2023

@pradyunsg should it be logged with DEBUG level instead of INFO, then? It's probably useful in some cases.

uranusjr
uranusjr previously approved these changes Sep 7, 2023
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEBUG or VERBOSE would make sense. I think I prefer DEBUG slightly, the state can be very verbose.

@uranusjr uranusjr dismissed their stale review September 7, 2023 07:03

DEBUG or VERBOSE would make sense (maybe DEBUG)

@pradyunsg
Copy link
Member

DEBUG works. :)

@webknjaz webknjaz force-pushed the bugfixes/resolvelib-debugging-reporter-ending-round-state branch from 62fae81 to 8b63fe6 Compare January 17, 2024 02:36
@webknjaz webknjaz requested a review from uranusjr January 17, 2024 02:36
@webknjaz
Copy link
Member Author

webknjaz commented Feb 3, 2024

@pradyunsg do you think this needs a change note?

@uranusjr
Copy link
Member

Personally I think this is fine, an additional log shouldn’t break things (you’re doing somethign wrong if it does…)

@uranusjr uranusjr added the skip news Does not need a NEWS file entry (eg: trivial changes) label Mar 26, 2024
@webknjaz
Copy link
Member Author

@pradyunsg it should be easy to merge this?

webknjaz added 2 commits July 15, 2024 12:14
This change corrects a tiny-little mistake made in
9f318de that was causing the word
"state" to be printed out in the log instead of the value of the
`ending_round()`'s state` argument.
@webknjaz webknjaz force-pushed the bugfixes/resolvelib-debugging-reporter-ending-round-state branch from 8b63fe6 to ea5f26e Compare July 15, 2024 10:14
@pradyunsg pradyunsg merged commit d9c6d68 into pypa:main Jul 15, 2024
28 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: cli Command line interface related things (optparse, option grouping etc) C: dependency resolution About choosing which dependencies to install C: error messages Improving error messages skip news Does not need a NEWS file entry (eg: trivial changes) type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants