Why is it then that I see gt 1000 lines of black linter call

Question

Why is it then that I see gt 1000 lines of black linter calls even when I don t touch python Do pull requests use a different individual cache

hundreds-father-404 · Answer

Because the CI job runs `build support bin ci py lint` which runs ` pants tag= nolint lint per target caching `

hundreds-father-404 · Answer

The effect is running Black once for every single target confused that s what ` per target caching` results in

fancy-queen-20734 · Answer

so `<http ci py|ci py> lint` doesn t use a cache in that case

hundreds-father-404 · Answer

Well CI uses a remote cache The remote cache will get the prior result of running Black on target 1 without having to do any new work But it will still output the result of running on target 1 The remote cache helps with the speed of that shard it does not help with the verbosity of output

fancy-queen-20734 · Answer

ah

fancy-queen-20734 · Answer

I see

hundreds-father-404 · Answer

It s not ideal We could in CI turn off ` per target caching` but that results in the shard taking about 10 minutes longer on average iirc because we re running on every single target The better solution is the upcoming project to remove ` per target caching` into a merged solution that gets the benefits of both

fancy-queen-20734 · Answer

We could pass ` quiet` to black in CI

fancy-queen-20734 · Answer

which would cause it to suppress output for non failures

hundreds-father-404 · Answer

Agreed Feel free to put up that PR if you have a moment and I ll approve slightly smiling face you d want to modify `pants travis ci toml`

fancy-queen-20734 · Answer

witty-crayon-22786 · Answer

i don t think it makes sense to show output on success for these linters i don t think yea black s output is cute but we re up to 4 linters and it s mostly just noise regardless of 1 run per target or 1 run for all targets

fancy-queen-20734 · Answer

yeah

fancy-queen-20734 · Answer

I feel like that s something that ought to be handled at the lint console rule level

fancy-queen-20734 · Answer

do console rules have common options around output

hundreds-father-404 · Answer

gt i don t think it makes sense to show output on success for these linters i don t think My main hesitation is that Pants is now introspecting the tools and modifying the results rather than us simply wrapping the tools That s an important philosophical distinction

witty-crayon-22786 · Answer

it doesn t need to modify it in any way just not render it on success

fancy-queen-20734 · Answer

yeah that

hundreds-father-404 · Answer

gt do console rules have common options around output No they don t beyond things like ` sep` and ` output file` iirc Also now renamed to goal rules

hundreds-father-404 · Answer

gt it doesn t need to modify it any way just not render it on success Sure tactically we can inspect the return code and not render if successful To do that we need to either in `black rules py` mutate the FmtResult so that the `stdout` attribute is empty or we need to generalize this filtering in `<http fmt py|fmt py>` Also when using ` no per target caching` I personally quite like Black saying ran over 71 files It s helpful when I m doing something like ` v2 changed since=master fmt` to know how many files I ve changed

witty-crayon-22786 · Answer

i think that this is general for all `lint` output

witty-crayon-22786 · Answer

we could perhaps differentiate success from warn and fail at the return code level and then render warn fail

hundreds-father-404 · Answer

This gets to Pierre s blog post Every tool handles return codes differently and things like warnings differently Maybe we do end up trying to create the unified interface he mentioned and I think implemented at Twitter But my point is that by us deciding to no longer render Black s stdout in some cases we are no longer a simple wrapper around underlying linters and formatters We are now in the business of interpreting the results For example Black has a ` verbose` option Now we are removing the ability for the user to run with ` verbose` on successful cases which they might want to do to check which precise files Black ran over

witty-crayon-22786 · Answer

it s less about individual tools and more about general consistency what we have done for things like ` verbose` in many other cases is propagate pants own log level to tools that are spawned

witty-crayon-22786 · Answer

because otherwise you need to know too many permutations of per tool flags to make progress

witty-crayon-22786 · Answer

and i don t think the goal is to be a simple wrapper it was in Pierre s case simple yes but consistent rather than thinly wrapped

hundreds-father-404 · Answer

Right we could do that My point is that we need to make a philosophical decision whether Pants continues to simply wrap formatters and linters or whether it has this unified interface we create In other words not showing Black s output when successful is less innocuous than it sounds It means that we ve decided we do want to get closer to Pants now having this unified interface

witty-crayon-22786 · Answer

the alternative is for 10 linters to output ten different formats very quickly it becomes unreadable noise

hundreds-father-404 · Answer

gt and i don t think the goal is to be a simple wrapper To clarify I argue that right now we are simple wrappers We don t put any restrictions on the passthru args you can give and we also allow you to specify a config file with whatever options you want On the extreme opposite we don t allow config files or passthrough args Everything is a Pants option

witty-crayon-22786 · Answer

i think that s a false dichotomy i m not suggesting adjusting which options are legal i m suggesting that pants should attempt to render the most useful information by default

witty-crayon-22786 · Answer

yes that s subjective

hundreds-father-404 · Answer

gt i think that s a false dichotomy i m not suggesting adjusting which options are legal We arguably would need some level of validating args though if we do this It likely no longer makes sense to allow users to specify ` black args= verbose ` and ` black args= quiet ` if Pants is already doing its own thing with output I m not saying we should absolutely avoid this only that I think we need to be really careful to open the door

witty-crayon-22786 · Answer

there is something to be learned from test running here i think <https github com pantsbuild pants blob bedf92358fb96e24f57a2f1183ccb395f0609c40 src python pants backend jvm tasks junit run py L94 L102>

hundreds-father-404 · Answer

Parametrizing this would be much better agreed

hundreds-father-404 · Answer

And it would be great to add this option to V2 test That pytest output is so ridiculously verbose in CI

witty-crayon-22786 · Answer

yes

witty-crayon-22786 · Answer

and then consistency comes back in if possible consistency across the goals would be good without overoptimizing for it

hundreds-father-404 · Answer

Cool parametrizing sounds good to me Although favor to please wait one day before anyone puts up PRs I m about to put up the Target API rewrite of `fmt` and `lint` which I ve been working on for 3 days and want to avoid merge conflicts

witty-crayon-22786 · Answer

i could imagine this reducing to <https pantsbuild slack com archives C046T6T9U p1586898277188700 thread ts=1586894810 183400 amp cid=C046T6T9U>

witty-crayon-22786 · Answer

in the sense that a tool would classify something as warn if it knew how

witty-crayon-22786 · Answer

and then at the top level parametization would decide what to do with process outputs

witty-crayon-22786 · Answer

very analogous to log levels i suppose

hundreds-father-404 · Answer

Probably more complex than only looking at return code E g Pytest will use a return code of 0 but print deprecations to stderr There we d inspect stderr to determine if there are warnings on success cases For Black it always prints to stderr no matter what No idea how to handle warnings there outside of regex matches on the stderr attribute which is icky

witty-crayon-22786 · Answer

yep

witty-crayon-22786 · Answer

well yes it s very tool specific

happy-kitchen-89482 · Answer

At the very least we need to add banners demarcating which output came from which tool

fancy-queen-20734 · Answer

yeah

happy-kitchen-89482 · Answer

Right now it s just a direct concatenation of the output from each tool which I find fairly confusing

fancy-queen-20734 · Answer

I think that would require tagging the results with the tool which right now they are not

hundreds-father-404 · Answer

That should be simple to do We already do it with Test for example We would add an attribute to `FmtResult` and `LintResult` for the name of the tool e g ` black ` vs ` flake8 ` Then we could add that to the output as a banner

fancy-queen-20734 · Answer

that and collating by tool would make for slick output

witty-crayon-22786 · Answer

mm This needs another round of design in general Because we re currently not rendering output until the end either And that s questionable for both tests and lint

fancy-queen-20734 · Answer

yeah

hundreds-father-404 · Answer

gt Because we re currently not rendering output until the end either I d love to change that but how is that possible We re using `await MultiGet` and we can t do anything with the results until it s all done I suppose the `coordinator` rule pattern

witty-crayon-22786 · Answer

Logging is the current way But if it isn t enough we d want to consider adding more native facilities