#development
Title
w
witty-crayon-22786
10/12/2022, 9:03 PM@hundreds-father-404: re: https://github.com/pantsbuild/pants/pull/17198
the bug (afaict) is that cache entries are written for failed processes
h
hundreds-father-404
10/12/2022, 9:03 PMwhich we can't control, that's up to each RE server implementation
it's why https://github.com/pantsbuild/pants/pull/17195 exists
w
witty-crayon-22786
10/12/2022, 9:04 PMsure: but that’s the impact of that flag: the server won’t check the cache
h
hundreds-father-404
10/12/2022, 9:04 PMand https://github.com/pantsbuild/pants/pull/17198 is the second part of it
w
witty-crayon-22786
10/12/2022, 9:05 PMthe first PR is correct, yea
h
hundreds-father-404
10/12/2022, 9:05 PMsure: but that’s the impact of that flag: the server won’t check the cacheyeah, if
--no-remote-cache-readw
witty-crayon-22786
10/12/2022, 9:06 PMOtherwise, it’s possible RE server will still look it up because we cannot control the behavior of that serverwe can: that’s what the flag is for, heh
h
hundreds-father-404
10/12/2022, 9:06 PMwhich is why I'm now setting the flag? which was not set before. I'm confused
w
witty-crayon-22786
10/12/2022, 9:06 PMbasically, i think that the flag should always be set in
remote::CommandRunnerh
hundreds-father-404
10/12/2022, 9:07 PMoh, yeah that could make sense
w
witty-crayon-22786
10/12/2022, 9:07 PMi think that it needs to be to actually fix the bug
h
hundreds-father-404
10/12/2022, 9:07 PMwhy would it need to be? seems only like a possible performance optimization to avoid duplicated work of both the server and Pants doing the same cache lookup
w
witty-crayon-22786
10/12/2022, 9:08 PMbecause the server doesn’t have our failure logic
h
hundreds-father-404
10/12/2022, 9:08 PMlike ProcessCacheScope?
w
witty-crayon-22786
10/12/2022, 9:08 PMright.
h
hundreds-father-404
10/12/2022, 9:08 PMyeah, that makes sense
okay
w
witty-crayon-22786
10/12/2022, 9:09 PMthanks: confusing.