https://pantsbuild.org/ logo
Join Slack
Powered by
so re: <https://github.com/pantsbuild/pants/issues...
# development
h
so re: https://github.com/pantsbuild/pants/issues/10650, what behavior do we want to happen when a started workunit is invalidated?
I thought about introducing a new 
WorkunitState::Canceled
, which would percolate into the 
StreamingWorkunitHandler
code
@polite-garden-50641 @witty-crayon-22786
w
good question.
generally “Canceled” is a form of “failure”
Completed/Started/Canceled might make sense. but Started/Succeeded/Failed, with cancellation being a particular message in Failed might also make sense.
👍 1
but it’s a better question for Asher and @fast-nail-55400 i think.
Asher as a consumer, and Tom as a producer of workunits
h
another question is, do we want to do anything with the log level when a node cancelation happens?
I noticed while testing my change that the workunits that get canceled are all Debug level, so the user won't normally see the message associated with their cancelation
we might want to bump the level to WARN
a node cancelation should be an exceptional state but not a failure state?
f
there are a few uses for workunits, right? tracing is one, and tracking and reporting results is another.
does cancelation remove a workunit from the graph?
h
@fast-nail-55400 yes, workunits are responsible for logging the "Started"/"Completed" messages in the console when pants runs, and are passed to plugins, including Toolchain's
no, the issue right now is that a 
Started
entry for a workunit will be emitted whenever a workunit starts
but there won't be a corresponding 
Completed
one
f
that would confuse me to not see some end to the story told by “Started”
better to just show me what work was done
h
so this means the workunit json handled by a plugin needs to handle the case where a completed workunit corresponding to a started one just never shows up, and also we'll log multiple "Started" lines without a "Completed" line
f
and the dynamic UI can show what is actively being worked on in the moment
h
I think it makes sense to log a "Canceled" message to the console, which is what my PR currently does
f
but what is doing the cancelation? speculation? or user ctrl-c’ing?
h
should be anything that causes the future associated with a node to not complete, which includes invalidating a node because the underlying files changed
f
using file changes as a specific example, telling me that work is canceled doesn’t add useful information . just say “files changed. recomputing graph” would be sufficient (if Pants were continually watching a tree). The same principle can be extended to other examples, does informing the user of cancelation add any new and useful info?
if ctrl-C is pressed, just let me know that everything is canceled in one line, don’t tell me about every single work item that Pants would have done. I as the user don’t care. I pressed Ctrl-C and that is that.
h
@fast-nail-55400 hm that's a good point, we might want to deliberately not log node cancellations due to ctrl-c
w
so those should see cancellation, and they are mostly Info
if they’re not seeing cancellation… then… that would be my bug.
h
@witty-crayon-22786 yeah no INFO-level nodes are getting canceled, so if we print a cancellation message at the same level as teh workunit, the user will only see them with -ldebug
w
@fast-nail-55400, @hundreds-breakfast-49010: this is primarily the “watching a tree case”… but i feel like rendering which processes were cancelled via control+c would not be unreasonable.
@hundreds-breakfast-49010: hm. weird. but yea, i can follow up on that independently. the primary thing that i think is important is accurate is the dynamic ui
and that consumes debug as well.
..hm, no. it uses with_workunits. weird!
h
what uses 
with_workunits
?
@witty-crayon-22786
w
BoundedCommandRunner
h
so in the dynamic UI case, if it's active we won't ever print a "Started" message, so the manifestation of a canceled workunit will be the absence of a "Completed" message for it. which is fine
with --no-dynamic-ui we'll see Started without Completed messages, so to make that clearer we might want to log a "Canceled" message at the same level
w
@hundreds-breakfast-49010: it’s not fine, because that workunit will just “run forever” in the dynamic ui
(that’s was the issue i observed that caused me to file the ticket)
h
I mean in terms of what to log, if anything, in the canceled case
w
got it.
Open in Slack
PreviousNext
Powered by