builder: utils.py print verbose information for run_command() - #1031
builder: utils.py print verbose information for run_command()#1031eshattow wants to merge 1 commit into
Conversation
7c030f8 to
ecf405a
Compare
ecf405a to
1bb6f80
Compare
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
07ed6be to
949d6bc
Compare
63b3b2a to
02b677a
Compare
| f"{Path(*Path(stack()[1].filename).parts[-2:])}:{stack()[1].function}" | ||
| " => " | ||
| f"{Path(*Path(stack()[0].filename).parts[-2:])}:{stack()[0].function}" |
There was a problem hiding this comment.
inspect.stack is a expensive function to call and we should only call it if we really need it. So either:
- we guard this print behind some check, that it will not be executed on each call
- use a better grouping without using the stack
- use debug instead grouping
There was a problem hiding this comment.
debug and guarding are not useful as there is no way to discover we are being run by GitHub Actions with steps debug or not. I think Python has lazy loading so that did not make sense either. It won't hurt anything to call stack only once per invocation, so applied, thanks!
There was a problem hiding this comment.
We currently don't need this code, we are fine to add it if it will only be used when the action is running with debug logs enabled. We don't want to slow down each execution for logs that we currently don't need. Please don't resolve a conversation if you don't agree with the requested changes
There was a problem hiding this comment.
@edenhaus writes:
We currently don't need this code, we are fine to add it if it will only be used when the action is running with debug logs enabled. We don't want to slow down each execution for logs that we currently don't need. Please don't resolve a conversation if you don't agree with the requested changes
(emphasis mine)
Sure, again, how exactly do you want that to be accomplished? I see no existing strategy that would allow checking within python code execution the caller GitHub Actions environment debug state, so are you having something in mind here how to do this ?
There was a problem hiding this comment.
I currently have no time to invest in this topic as our CI is working fine without any issues. We will not merge something that will slow down our CI if we don't need it. Therefore please come up with a proposal, that this debug code is just executed when enabled/needed. I thought about using the debug functionality of GitHub actions as it sounds like a good fit.
0b3fdc3 to
341b22f
Compare
341b22f to
e427f2b
Compare
|
rebased to master |
|
Drafting as requested changes are not implemented |
2f9c68f to
0ba3030
Compare
Let's expose inspect information about the caller and callee filename and function name in the CI/CD output for wheels builder python runtime, and the non-static arguments presented to subprocess.run() * output is grouped instead of debug-only as a balance between verbosity and being noisy * output gates printing env if not empty or None due to concerns about leaking secrets via env * static arguments to subprocess.run() are not presented Co-Authored-By: Robert Resch <robert@resch.dev>
0ba3030 to
f7e8577
Compare
3 participants
builder: utils.py print inspect information for run_command() as group
Let's expose inspect information about the caller and callee filename and
function name in the CI/CD output for wheels builder python runtime, and
the non-static arguments presented to subprocess.run()
output is grouped instead of debug-only as a balance between verbosity
and being noisy
output gates printing env if not empty or None due to concerns about
leaking secrets via env
static arguments to subprocess.run() are not presented
Resolves: #1036
Co-Authored-By: Robert Resch robert@resch.dev