https://pantsbuild.org/ logo
w

witty-crayon-22786

10/12/2022, 9:03 PM
the bug (afaict) is that cache entries are written for failed processes
h

hundreds-father-404

10/12/2022, 9:03 PM
which we can't control, that's up to each RE server implementation
w

witty-crayon-22786

10/12/2022, 9:04 PM
sure: but that’s the impact of that flag: the server won’t check the cache
h

hundreds-father-404

10/12/2022, 9:04 PM
w

witty-crayon-22786

10/12/2022, 9:05 PM
the first PR is correct, yea
h

hundreds-father-404

10/12/2022, 9:05 PM
sure: but that’s the impact of that flag: the server won’t check the cache
yeah, if
--no-remote-cache-read
, then both the server will be instructed to not do its own internal remoet cache lookup, and also our own RemoteCache commandrunner won't do a lookup. Otherwise, it's possible RE server will still look it up because we cannot control the behavior of that server
w

witty-crayon-22786

10/12/2022, 9:06 PM
Otherwise, it’s possible RE server will still look it up because we cannot control the behavior of that server
we can: that’s what the flag is for, heh
h

hundreds-father-404

10/12/2022, 9:06 PM
which 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 PM
basically, i think that the flag should always be set in
remote::CommandRunner
, regardless of `cache-read`/`cache-write`, because we are doing our own cache reads/writes
h

hundreds-father-404

10/12/2022, 9:07 PM
oh, yeah that could make sense
w

witty-crayon-22786

10/12/2022, 9:07 PM
i think that it needs to be to actually fix the bug
h

hundreds-father-404

10/12/2022, 9:07 PM
why 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 PM
because the server doesn’t have our failure logic
h

hundreds-father-404

10/12/2022, 9:08 PM
like ProcessCacheScope?
w

witty-crayon-22786

10/12/2022, 9:08 PM
right.
h

hundreds-father-404

10/12/2022, 9:08 PM
yeah, that makes sense
okay
w

witty-crayon-22786

10/12/2022, 9:09 PM
thanks: confusing.