https://pantsbuild.org/ logo
Join Slack
Powered by
does anyone know (at a high level) what `reset_log...
# development
h
does anyone know (at a high level) what 
reset_log_location
https://github.com/pantsbuild/pants/blob/master/src/python/pants/base/exception_sink.py#L144 was meant to do at a high level?
the docstring says "Re-acquire file handles to error logs based in the new location.", but I'm not sure what the new location its talking about is; or in general to what extent this is out of date compared to how pants logging works today in a rust context
h
@aloof-angle-91616 do you remember? https://github.com/pantsbuild/pants/pull/6552 looks relevant
It’s 2 year old code. And the Rust logging is ~1 year old thanks to Borja’s changes. So it sounds very plausible that it is out of date
h
it looks like this code is responsible for creating the 
.pids
directory in the workdir
which I haven't generally looked at
was that directory called 
.pids
because it want to have thread-specific debug files back when pantsd was multithreaded?
h
That I don’t know. I know that generally there was a reason to not have 
.pids
as a part of 
.pants.d
because of the logic with cleaning 
.pants.d
, iirc. https://github.com/pantsbuild/pants/issues/3511 might have some helpful context
h
yeah I think there's a broader question of what behavior we actually want out of logging to 
.pids
w
signal handlers have constraints on what they are able to do
one thing that they can’t do is open files
so to interact with a file in a signal handler, you need to already have a file handle for it open
h
@witty-crayon-22786 so ExceptionSink should be keeping around handles to open files, so signal handlers can write to them?
this is the 
SignalHandledNonLocalExit
mechanism I think?
w
this is the 
SignalHandledNonLocalExit
 mechanism I think?
maybe, i don’t know.
h
anyway we call 
reset_log_location
exactly once when we start pants, and never invoke it again
I don't know if that's always been the case
w
it used to have more calls i think.
h
well actually no, we call it once in pants_runner, once in pants_daemon, and once in the exception sink module itself
that last one might be redundant
w
i don’t think it is… it needs to be initialized
h
anyway the reason i was looking at this was because the test that's sporadically failing in https://github.com/pantsbuild/pants/pull/10758 is one of the ones that was meant to test the 
.pids
logging functionality, I believe
I was trying to see if we could just get rid of that test, but I think it's testing functionality we do actually want
Open in Slack
PreviousNext
Powered by