#development
Title
h
hundreds-breakfast-49010
08/19/2020, 10:58 PMso 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::CanceledStreamingWorkunitHandler@polite-garden-50641 @witty-crayon-22786
w
witty-crayon-22786
08/19/2020, 11:01 PMgood 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
hundreds-breakfast-49010
08/19/2020, 11:04 PManother 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
fast-nail-55400
08/19/2020, 11:18 PMthere 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
hundreds-breakfast-49010
08/19/2020, 11:19 PM@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
Startedbut there won't be a corresponding
Completedf
fast-nail-55400
08/19/2020, 11:20 PMthat would confuse me to not see some end to the story told by “Started”
better to just show me what work was done
h
hundreds-breakfast-49010
08/19/2020, 11:20 PMso 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
fast-nail-55400
08/19/2020, 11:21 PMand the dynamic UI can show what is actively being worked on in the moment
h
hundreds-breakfast-49010
08/19/2020, 11:21 PMI think it makes sense to log a "Canceled" message to the console, which is what my PR currently does
f
fast-nail-55400
08/19/2020, 11:22 PMbut what is doing the cancelation? speculation? or user ctrl-c’ing?
h
hundreds-breakfast-49010
08/19/2020, 11:23 PMshould be anything that causes the future associated with a node to not complete, which includes invalidating a node because the underlying files changed
f
fast-nail-55400
08/19/2020, 11:25 PMusing 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
hundreds-breakfast-49010
08/19/2020, 11:48 PM@fast-nail-55400 hm that's a good point, we might want to deliberately not log node cancellations due to ctrl-c
w
witty-crayon-22786
08/20/2020, 12:01 AMhttps://pantsbuild.slack.com/archives/C0D7TNJHL/p1597878287141500?thread_ts=1597877887.139300&cid=C0D7TNJHL there will be processes involved
so those should see cancellation, and they are mostly Info
if they’re not seeing cancellation… then… that would be my bug.
h
hundreds-breakfast-49010
08/20/2020, 12:04 AM@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
witty-crayon-22786
08/20/2020, 12:04 AM@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
hundreds-breakfast-49010
08/20/2020, 12:27 AMwhat uses
with_workunits@witty-crayon-22786
w
witty-crayon-22786
08/20/2020, 12:27 AMBoundedCommandRunner
h
hundreds-breakfast-49010
08/20/2020, 12:29 AMso 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
witty-crayon-22786
08/20/2020, 12:31 AM@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
hundreds-breakfast-49010
08/20/2020, 12:33 AMI mean in terms of what to log, if anything, in the canceled case
w
witty-crayon-22786
08/20/2020, 12:34 AMgot it.