https://pantsbuild.org/ logo
Join Slack
Powered by
if I want to add a ‘[cached]’ label on the “Comple...
# development
f
if I want to add a ‘[cached]’ label on the “Completed: Run …” message for remote executions, what is suggested way to do that?
or does Pants have infra to collect metrics counts for a run?
h
cc @hundreds-breakfast-49010
h
@fast-nail-55400 that line of code is executed from the 
workunit_store
crate, look for 
log_workunit_state
f
context is getting an explicit way to see if remote execution caching is working in CI environment
h
so that sounds like a bit of state we'd have to make available on the workunit
probably on 
WorkunitMetadata
specifically, just add another boolean
f
would it be better as a vec of string labels?
h
possibly; if we think we might want to make this more general later
f
cc @polite-garden-50641 ^^
h
Tom, maybe print a debug message for now? Then you avoid the design question of what we should show at INFO level to users when it was remote execution vs local For example, a user has already requested that we log when something comes from the local cache. So something like 
[cached]
in the message might be confusing
👍 1
f
debug mode changes the pex logging on the remote
which changes what is being cached
👍 1
h
Is this for local iteration, or you want to commit the change?
f
I want to commit some form of this change so I can monitor how CI builds are operating
👍 1
and eventually for repos using toolchain buildsense, get metrics uploaded in the run data upload
h
the high-level job of that log method is to print "Completed: <workunit name>" whenever a workunit is done
so on second thought, we might deliberately want to avoid adding more and more debug information into it
i.e. this should be a log statement instead
unless the idea is that 
[cached]
should be part of the name of the remote process workunit
h
Okay. I think a simple change for now could be to do something like “Completed (via RE) - run Pytest” I think the issue with 
[cached]
is that it implies caching only happens with RE. Also, it implies that the result was read from the cache, even if it had to run fresh
f
well then I want a way to just enable log statemets for a particular logger and not have remote pex debug logging turned on
h
I have a half-completed commit for selective log levels
h
well then I want a way to just enable log statemets for a particular logger
Stu really wants this too and has been putting some thinking into it. But not a 2.0 priority, so I’d go with the hacky approach for now
Oh nvm, didn’t realize Greg was working on it 🙂
h
and yeah not working on it right this second becuase it's less of a 2.0 priority
f
as I said, affecting the REv2 protos changes what is cached and doesn’t let me turn on logging in a subsequent run to see what happened exactly in a prior run
w/o basically making the cache (the thing I’m investigating) useless
h
does changing pants' logging level affect the remote execution protobufs?
the remote pex debug thing is a different pants option, right?
h
the remote pex debug thing is a different pants option, right?
No I don’t think so. A global 
-ldebug
causes us to pass 
-vvv
to the Pex run, which Tom is saying is an issue
f
beats me, 
-ldebug
enables it everwhere
h
aah ok
f
-ltrace
is even worse with the logging volume
👍 1
esp. from the remote where I don’t want it
h
yeah, I don't think -ltrace is useful right now
b/c of the volume
seems like we'd want to be able tomake -ldebug not affect the pex run, precisely to avoid creating a new protobuf
I think we're talking about potential changes to pants that won't solve tom's immediate problem though
f
may I add an log-level option to the pex subsystem that would be set separate from the main log level?
and then don’t propagate the main log level through if it is set?
h
that seems reasonable to me
it make sense that pex logging should be in principle a separate set of options form pants logging, even if we try to be smart about setting pex logging automatically from pants logging
f
okay will code that up
Open in Slack
PreviousNext
Powered by