https://pantsbuild.org/ logo
Join Slack
Powered by
re: <https://github.com/pantsbuild/pants/pull/1236...
# development
w
re: https://github.com/pantsbuild/pants/pull/12369 : the change in log messages to render “Hit local/remote cache for” is only obvious for processes which already had a level of `Info`: the level wasn’t being respected before
cc @happy-kitchen-89482, @hundreds-father-404
i’d like to get that logging to a place that people are happy with before landing, because it’s going to be cherry-picked to two branches
h
Thanks for flagging as a Slack thread To be clear, if you are not using Pantsd, then you'll get a log message that 
pytest.pex
was built every single time you run Pants, right? Along with requirements.pex, pytest_runner.pex, etc
w
…it looks like the default is currently INFO: https://github.com/pantsbuild/pants/blob/ddc3c3d791178a610a9b8988cb74ac072bdac9c0/src/python/pants/engine/process.py#L78 , so probably step one is changing that to DEBUG, and then raising only a few.
To be clear, if you are not using Pantsd, then you’ll get a log message that 
pytest.pex
 was built every single time you run Pants, right? Along with requirements.pex, pytest_runner.pex, etc
you’ll get a message like 
[INFO] Completed: Hit local cache for: Build pytest.pex
, yes. because those processes are at level 
INFO
.
h
I think INFO is the right level though - resolving requirements can be slow AF, and in CI, you want to see how long it's taking But that's frustrating to have every time you run something on desktop, where we local cache hits to be frequent
To be clear, is this change technically necessary to fix the underlying bug?
w
moving to rendering this workunit rather than the workunit that was below the BoundedCommandRunner was necessary, yes.
but the fact that this workunit is using the Process.level now is a somewhat independent bugfix: it used to be hardcoded at trace
h
If folks aren't interested in this, we can have this same block lower the level of the workunit to debug.
Can this be lowered to Debug based on if it was a cache hit vs ran?
w
it can, but you’d still see 
Starting
at the Process’s configured level when 
--no-dynamic-ui
h
Hm, and is that unavoidable? As in we can't get back the old behavior
w
i think that we can get back to rendering at INFO only when we actually run the process. and now that i think about it, we’ll need to… just realized there will be an issue with the Starting messages with this change.
🙌 1
h
Excellent, I do think that's the preferred behavior. And for users who want to know when stuff is cached, I think the dynamic UI and BuildSense / 
--stats-log
is the way to do that
Open in Slack
PreviousNext
Powered by