#development
Title
h
hundreds-father-404
07/31/2020, 8:55 PMHow do you get Pants to not output when a
Processw
witty-crayon-22786
07/31/2020, 9:03 PMwe don’t have a way to set the “level” of a process… might be a quick fix.
they should be cached though: you shouldn’t see them again until you change the PATH
but, related: i realized yesterday that they’re buggy for that reason. will need to do more of #9760 and make the path lookup operation native as described there.
will open a new issue for that.
h
hundreds-father-404
07/31/2020, 9:07 PMhttps://github.com/pantsbuild/pants/pull/10525 for the first step of improving this.
they should be cached thoughYes, but I also think this is a dangerous mindset that we need to remind ourselves to watch out for. The first time experience of a user matters. For a first time user, are these particular messages pulling their weight? I think no.
we don’t have a way to set the “level” of a process… might be a quick fix.Would this be adding a property to
Processlog levelw
witty-crayon-22786
07/31/2020, 9:13 PMwill open a new issue for that.https://github.com/pantsbuild/pants/issues/10526
@hundreds-father-404: just “level”, yea.
(because it’s also the workunit’s level)
👍 1
h
hundreds-father-404
07/31/2020, 11:58 PMDo we think the better default for processes is debug or info? That is, should we default a process to logging “Completed: foo”? cc @happy-kitchen-89482
h
happy-kitchen-89482
08/01/2020, 12:01 AMHmmm
h
hundreds-father-404
08/01/2020, 12:01 AMSo far, I’m changing these to debug:
- run
protocarchive.pyclocpythonh
happy-kitchen-89482
08/01/2020, 12:01 AMProbably yes?
h
hundreds-father-404
08/01/2020, 12:01 AMI’m atm keeping these as info:
- building a Pex
- running a Pex process, like
pytestI think defaulting to info makes sense? Personally, though, I don’t like how chatty Pants is, so my fear with a default of info is that when we add new code, it will risk being chattier than it should be
w
witty-crayon-22786
08/01/2020, 12:04 AMInfo is a better default.
👍 1
until https://github.com/pantsbuild/pants/issues/7907 is implemented, a @rule cannot have a non-static
descriptionwhich means that right now only processes have “useful” descriptions.
h
hundreds-father-404
08/01/2020, 12:05 AMAgreed, but even if the level is set to Debug, it will still show up in the dynamic UI, right?
The ideal imo would be to show up in the dynamic UI, but not be persisted as a log message.
w
witty-crayon-22786
08/01/2020, 12:06 AMAgreed, but even if the level is set to Debug, it will still show up in the dynamic UI, right?yea, true.
my feeling is that running a process is almost always a significant thing. some of the ones you’ve mentioned above should stay Info, IMO
the PATH discovery stuff is pretty clearly not.
h
hundreds-father-404
08/01/2020, 12:07 AMclocclocw
witty-crayon-22786
08/01/2020, 12:08 AMthat goal does, not all will. but yea, maybe a special case.
h
hundreds-father-404
08/01/2020, 12:09 AMOverall, I think I agree with you that Info is a better default. I’ll stick with it. Thanks!
w
witty-crayon-22786
08/01/2020, 12:11 AMoh, so: another consideration.
nevermind!
(was gonna say “we’re considering conditionally dumping output on processes and that might be level driven” but that’s not true… that ended up in the EngineAware interface)
… i had almost forgotten about that.
that exists now, so if you want streaming test output for failures…
…can do it in a few lines 😃
❗ 1
… i had almost forgotten about that.not even almost: “fully forgotten”.
tweak
TestResultdef message(thanks @hundreds-breakfast-49010, wherever you are)
👍 1
h
hundreds-father-404
08/01/2020, 12:16 AMNeat. It sounds like that’s a separate change, though, right?
w
witty-crayon-22786
08/01/2020, 12:16 AMdefinitely
if i had remembered i would have suggested doing it in https://github.com/pantsbuild/pants/issues/10484
👍 1
…need to not forget again. filing a ticket.