Add logging verbosity level flag to extract_ir.py #278
+12
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
https://abseil.io/docs/python/guides/logging? has already a command line for verbosity (should be |
boomanaiden154
force-pushed
the
extract-ir-logging-verbosity
branch
from
adaa73d
to
2e8454e
Compare
|
Updated to use the absl logging facilities rather than custom plumbing everywhere. |
mtrofin
reviewed
Jul 19, 2023
This commit adds a logging verbosity level flag to extract_ir.py and the appropriate plumbing/implementation in extract_ir_lib.py. This is primarily motivated by these errors coming up quite often in non-trivial builds (as it is fairly often there is some assembly or some flags don't get passed around somewhere) and not providing a very high signal to noise ratio, especially when used as a library against a bunch of projects at once.
boomanaiden154
force-pushed
the
extract-ir-logging-verbosity
branch
from
2e8454e
to
c7e2903
Compare
mtrofin
reviewed
Jul 20, 2023
mtrofin
reviewed
Jul 20, 2023
- Don't change logging verbosity back to info to print the status message if the verbosity has been set lower.
- Log subprocess output directly rather than having subprocess write directly to STDOUT/STDERR.
- Switch to using subprocess.check_output
mtrofin
approved these changes
Jul 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds a logging verbosity level flag to extract_ir.py and the appropriate plumbing/implementation in extract_ir_lib.py. This is primarily motivated by these errors coming up quite often in non-trivial builds (as it is fairly often there is some assembly or some flags don't get passed around somewhere) and not providing a very high signal to noise ratio, especially when used as a library against a bunch of projects at once.
I've implemented this as a custom flag rather than using absl's
logging.get_verbosityandlogging.set_verbosityas I think this approach is a bit more expressive for the purposes here, and it also avoids us needing to set global state which is nice within the project and especially for downstream users (although it's not a big switch to do so the other method). I'm open to switching over if others have different ideas.