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

witty-crayon-22786

12/09/2020, 7:15 PM
reviving a previous discussion about the caching of interpreter lookups for python
👍 1
we currently try not to, but there was a bug in UUIDRequest (pr coming soon) that was breaking
--test-force
fixing the bug in UUIDRequest (would like to port that into a native implementation so that it is less error prone) adds ~1.2seconds to a noop
test
run (from 1.2 to 2.4)
👍 1
the 1.2 seconds is in re-running tool discovery per-run (https://gist.github.com/stuhood/5152908bd9953424d6fd2356010f13de)
while it is technically correct to do (until https://github.com/pantsbuild/pants/issues/10769 i think?)… that’s quite a cost to pay.
cc @enough-analyst-54434
h

hundreds-father-404

12/09/2020, 7:28 PM
I wonder if we do want to make it something manual..like a
./pants invalidate-binaries
I originally thought no way to manual. It's way too error prone and easy to not know how to do. But the reality is that it seems very rare for something like /usr/bin/tar to change. Especially in CI, it's very rare to change. Even if we can lower the cost, doing it automatically will still add overhead
w

witty-crayon-22786

12/09/2020, 7:29 PM
right now, a manual step would be to kill
pantsd
or edit
pants.toml
to force a restart
h

hundreds-father-404

12/09/2020, 7:30 PM
Yeah, but I'm saying we never re-search the binaries unless a) the discovery process failed, or b) the user runs a command. John and my original intent was to always automatically rerun every pantsd session. Even if we can lower that cost, it's still expensive to rerun
h

happy-kitchen-89482

12/09/2020, 7:32 PM
Yeah, that is too high a time price to pay for hypothetical correctness
h

hundreds-father-404

12/09/2020, 7:34 PM
Well, it is important for correctness. If you install a new interpreter with Pyenv, or upgrade your OS, for example, pants will not detect the change until you run a manual command. That's really easy to forget and a bad UX But I'm suspecting the even worse UX is for pants to waste a bunch of time doing something that 99% of the time is irrelevant
👍 2
w

witty-crayon-22786

12/09/2020, 7:37 PM
or upgrade your OS
that is a restart, so pantsd won’t survive it, but yes to the other thing
h

hundreds-father-404

12/09/2020, 7:43 PM
Right, I am saying that even the "per pantsd session" invalidation might be too costly and we should never automatically invalidate
w

witty-crayon-22786

12/09/2020, 7:44 PM
i’m not 100% sure that we’re all using
Session
the same way. a
Session
is a single run of pants
there isn’t a formal name for “the span of time that
pantsd
is up”, afaik
h

hundreds-father-404

12/09/2020, 7:45 PM
We probably aren't. The current behavior for discovery of binaries should be "per pantsd lifetime". Test force should be "per pants run"
w

witty-crayon-22786

12/09/2020, 7:47 PM
got it. yea, that is not happening. we’re re-running both per-
Session
.
i’ll see if i can adjust https://github.com/pantsbuild/pants/pull/11288 to do that without going deeper in this PR.
👍 1
h

hundreds-father-404

12/09/2020, 7:49 PM
I believe this all once worked, when John originally added it. This sounds like a regression
👍 1
w

witty-crayon-22786

12/09/2020, 7:50 PM
my hunch is that it worked for either interpreter discovery or for
--test-force
, but not both at the same time.
h

hundreds-breakfast-49010

12/09/2020, 7:58 PM
how hard is https://github.com/pantsbuild/pants/issues/10769 to do? if we can get that in quickly, and that solves the problem conceptually correctly, then it doesn't matter much which decision we make with the --test-force PR
w

witty-crayon-22786

12/09/2020, 7:58 PM
i started the thread thinking that we currently wanted to re-run discovery for every pants run. if that’s not the case, then this isn’t that bad (that behavior was working, so it just needs to be preserved in #11288)
h

hundreds-breakfast-49010

12/09/2020, 8:00 PM
ideally, we would be able to determine if the binary changed under us very quickly, so we could in fact run that check on every pants run
w

witty-crayon-22786

12/09/2020, 8:01 PM
it’s harder than you might think. take a look at the gist to see how much might change under us: https://gist.github.com/stuhood/5152908bd9953424d6fd2356010f13de
👍 1
h

hundreds-father-404

12/09/2020, 8:03 PM
ideally, we would be able to determine if the binary changed under us very quickly, so we could in fact run that check on every pants run
I disagree with every run. It rarely changes, and doesn't seem too offensive to me to have to restart pantsd if you install a new interpreter. But, would be good to make it easier to force restart pantsd
h

hundreds-breakfast-49010

12/09/2020, 8:03 PM
I think the concern is more not realizing that you have to restart pantsd
I agree that tool changes should happen rarely enough that it's not a problem if there's a manual step involved
w

witty-crayon-22786

12/09/2020, 8:04 PM
@hundreds-father-404: to be clear, i agree that we should fix #10769. i just don’t think it’s urgent.
h

hundreds-breakfast-49010

12/09/2020, 8:05 PM
but yeah I would say that for right now, it's better to be technically incorrect in a way that causes silent failures occasionally than waste 1.2 seconds every time. it sounds like stu's PR as it currently stands ends up doing the latter
ok, i started this thread on a false premise: let's close it out and start over for further discussion: sorry!