https://pantsbuild.org/ logo
#development
Title
# development
w

witty-crayon-22786

07/19/2021, 8:22 PM
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

hundreds-father-404

07/19/2021, 8:24 PM
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

witty-crayon-22786

07/19/2021, 8:25 PM
…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

hundreds-father-404

07/19/2021, 8:26 PM
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

witty-crayon-22786

07/19/2021, 8:28 PM
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

hundreds-father-404

07/19/2021, 8:29 PM
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

witty-crayon-22786

07/19/2021, 8:31 PM
it can, but you’d still see
Starting
at the Process’s configured level when
--no-dynamic-ui
h

hundreds-father-404

07/19/2021, 8:34 PM
Hm, and is that unavoidable? As in we can't get back the old behavior
w

witty-crayon-22786

07/19/2021, 8:37 PM
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

hundreds-father-404

07/19/2021, 8:38 PM
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