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

hundreds-breakfast-49010

12/12/2019, 1:58 AM
https://github.com/pantsbuild/pants/pull/8799 <- curious what people think of this solution to https://github.com/pantsbuild/pants/issues/8062 , particularly @red-balloon-89377 @witty-crayon-22786 @happy-kitchen-89482
the other possible solution (which I haven't yet coded up yet) would be to put what I'm calling `printable_msg`s and the existing logs into the same deque, print them in the space above the swim lanes as they are emitted, and hopefully when the swim lanes go away the interleaved log/console output would make sense
tangentially-related: I just noticed that when we print logs in v2, I think their timestamps are in UTC rather than localtime
and in general we might want to think about what text format we use for logs in v2
w

witty-crayon-22786

12/12/2019, 2:45 AM
@hundreds-breakfast-49010: that seems like a good solution!
... i think that the only other potential solution that feels reasonable would be to abort rendering the UI upon the first stdout/stderr?
it might provide better latency?
but maybe not much better
cc @red-balloon-89377: ^
r

red-balloon-89377

12/12/2019, 4:18 PM
Will review before EOD today, can’t right now
w

witty-crayon-22786

12/12/2019, 4:19 PM
np
r

red-balloon-89377

12/12/2019, 5:06 PM
That approach looks good to me for now, only beacuse we have agreed to log test failures preemptively in addition to printing them at the end.
However, I think users would still like to see why their tests failed when they fail, so logging the output of the test runner would be good as a convention. I know it’s kind of orthogonal, but I feel that agreeing on this convention is very important for this model to work.
I’m half-tempted to propose a change in
Console
to have two ways of writing something to stdout/err: One that’s buffered, which should be the default and is what’s described in #8799, and one that’s not buffered, which would implement the alternative that Greg proposed.
That way, console-rule authors can choose whether they want the output to be immediate, or parseable by a tool.
w

witty-crayon-22786

12/12/2019, 5:43 PM
@red-balloon-89377: they would not be rendered to stdout/stderr immediately regardless though, since the console_rule would be suspended until all tests had completed
r

red-balloon-89377

12/12/2019, 6:10 PM
That's a good point :) then logging the test output is starting to sound better and better
h

hundreds-breakfast-49010

12/12/2019, 7:24 PM
@red-balloon-89377 if we're going to decide that will both log some things and print them out explicitly, then I see that as an argument against trying to interleave logging and normla printing
r

red-balloon-89377

12/12/2019, 7:26 PM
I don't think those would be the same things. For instance, I might want to notify the user of a test failure ASAP, whereas for something like
pants export
I only want the guarantee that the JSON I print at the end will be printed all at once
What it looks like is that the first case (immediate feedback) we would exclusively solve with logging,use printing to stdout/err only for the second case
👍 1
Which is fine by me
h

hundreds-breakfast-49010

12/12/2019, 7:30 PM
I wonder if it makes sense to extend the use of the log infrastructure for "anything that we want to be printed immediately"
I tend to think of a log as something that outputs information about how a program is running which isn't necessarily useful for the person running the program
so, it's a little odd if we're using pants logging to display information that is legitimate program output (like a test failure), just stuff we want to see immediately
w

witty-crayon-22786

12/12/2019, 7:46 PM
i'm less sure about that... i think that a better definition of a line here might be "things that i might want to parse" (console output) vs "execution information" (logging)
h

hundreds-breakfast-49010

12/12/2019, 7:47 PM
hm, ok
w

witty-crayon-22786

12/12/2019, 7:48 PM
in general, we expect pants to be run at
info
log level, because things relevant to what you are doing will be rendered at info
(having said that, we want drastically less log output than we had in v1, so we'll want to review carefully for logging at info that shouldn't be)
h

hundreds-breakfast-49010

12/12/2019, 7:49 PM
one of the things I noticed we're not doing correctly right now, is that logging via the EngineDisplay abstraction is ignoring the metadata about a log item
which is something I was going to fix
but, okay, if we go with the "things I'm going ot parse" understanding of what Console is doing, then I think this commit is fine
w

witty-crayon-22786

12/12/2019, 7:50 PM
oh!... i forgot an important aspect here: console output is "always" rendered, even during cache hits. but logging isn't
h

hundreds-breakfast-49010

12/12/2019, 7:50 PM
and if we expect users to want to see certain things (such as test output) immediately, then it will show up with log metadata
ah, good point
w

witty-crayon-22786

12/12/2019, 7:51 PM
so even if we render test failures in some form in the log (we should), then we'd also want to render them as part of the console output
h

hundreds-breakfast-49010

12/12/2019, 7:51 PM
so right now a failing test is a thing that's perfectly cacheable, and we're working on that flag we were talking about ot make running a specific test locally and interactively easy to do
@witty-crayon-22786 right that makes sense
so the only difference would be when exactly the printing happens
in the first run case with no caching
anyway, it sounds like what i'm doing with deliberately buffering print lines and waiting until the end to print them does solve theproblems we care about
w

witty-crayon-22786

12/12/2019, 7:52 PM
yea, works for me. thanks!
h

hundreds-breakfast-49010

12/12/2019, 7:52 PM
cool, I'll flip the PR from draft to, uh, real I guess
"ready for review" that's the phrasing they use