https://pantsbuild.org/ logo
Join Slack
Powered by
<https://pantsbuild.slack.com/archives/C15HUUSL9/p...
# development
a
a good thread from stu. these are all things i’d like to experiment with and having e.g. a table of intrinsics as stu describes would be one way to get a broader understanding of the syntax we do have already and how it interrelates
i was thinking that one way to implement retries would be to e.g. allow wrapping any 
yield Get(A, B, c)
“subject” (aka 
c
) in a 
RetryableRequest
object, which would wrap the “real” 
c
and add some backoff/retry info, and which the engine would interpret as “perform this yield Get() and apply the specified backoff strategy if it fails”
stu’s concern was that this introduced new syntax (which he defines above)
i also wanted to do something else that fell under that definition of new syntax, aka using 
@union
classes as “protocols” of a sort, which meshes incredibly well with python 3.8’s `typing.Protocol`: https://github.com/pantsbuild/pants/pull/8542
so there are a couple approaches i’d like to take in this thread, and i think they’re very forwards-compatible, but there are concerns about whether this introduces new syntax. i’d like to discuss what type of changes to the engine API we’d like to avoid, and what to do instead. stu and i both have answers to this question in this thread — would love to hear others’ thoughts
w
teachability/explainability is a big factor, i think
fewer/simpler concepts have a benefit there.
h
so, intrinsics are themselves "just types", but they're types in rust
in a long-term view, it would be nice if rules could be written in rust as easily in python, and have similar notions of types
one thing I'm not super-clear on, having hacked around in there a bit now, is how pants should determine whether a rule needs to be an intrinsic or not. intrinsics are already a bit of a special case, in my way of thinking anyway
@aloof-angle-91616 I think I agree that using additional UnionRule-like python classes isn't really new syntax
I remember when I was learning how pants worked, I had trouble understanding the point of UnionRules. and I actuallly wonder if having an additional type of "pseudo-superclass" might make the concept easier for newcomers to grok
"`@union` makes a type one of a class of equivalent types, 
@retryable
makes the engine retry up to n times if the first time it gets this type something breaks", etc.
w
i think that in the case of test retry, the syntax is potentially less general. i can think of lots of things you can build with the ability to sleep
retry is one of them
_maybe_/probably those other usecases are less important
a
on the other hand, the comparison i might make is providing mutexes in a programming language versus futures. we could allow for sleep and write code that looks like C, i was just thinking that we could introduce higher-level operations and the argument against it is that it’s extending syntax. i don’t understand why we want the more general sleep function when all we want here is test retry
w
that brings us back to the syntax though. if the more general/powerful concept also does not require something new to teach (it's another thing in the table of intrinsics), is it better or worse?
a
i also don’t see how using 
yield Get(_, Sleep(2))
is composable
w
what do you mean?
a
how could i write a rule that performs exponential backoff when executing another rule except by manually implementing it in each rule?
a RetryableRequest intrinsic can be extended to support other strategies, just by modifying the rust code that processes it. a Sleep intrinsic seems like it would require a lot of boilerplate in every method to use effectively.
w
it's definitely possible to implement 
RetryableExecuteProcessRequest
or something from python given the ability to sleep
a
i see one of the positives of the v2 engine as abstracting execution from rules themselves. i don’t feel as comfortable introducing concepts such as Sleep which moves a lot of that code that manages execution into python all of a sudden
w
generically retrying "any rule" with backoff is harder. but i'm unsure about how useful that is
i think that test retry is an interesting beast in this regard. i'm not sure how many places we will need exponential backoff. i'm not even sure we need it for test retry, honestly?
in fact, maybe i'm totally wrong and we should bake it into ExecuteProcessRequest as an argument
(also probably lighter than syntax.)
a
i just don’t see why we need that limitation. i can put up a draft PR showing how the generic backoff might be implemented. would that be useful here?
(phone in airplane mode again — sorry. might hack something up on the plane. i’m not too opinionated about this, just trying to discover the space here)
w
mm, not sure it's a priority... i don't think it's necessary for the first version
if you want to post a gist at some point?
✍️ 1
thank you though.
❤️ 1
a
i can post a branch so that it’s working code
@witty-crayon-22786 2 questions: (1) would it be more palatable to add more types that are known to the engine (e.g. a generic 
RetryableRequest
) if, instead of having to modify the FFI each time we add an intrinsic, we added types such as 
Snapshot
and 
RetryableRequest
to a dict (a literal table of intrinsics) which would then be converted into a rust hashmap and accessed by name? (2) creating this table of intrinsics that gets passed to rust and DRYing it up could be the first change, completely separate from (1) or trying to add anything new to what gets passed to rust (which would be a separate discussion). does that sound like it could get closer to what you're describing in this thread (formalizing the "table of intrinsics")?
w
there is probably boilerplate that can be removed around intrinsics, yea. but the definition of an intrinsic is that it has an implementation in rust
so there is only so much boilerplate you can remove and still align the python side with the rust side
less boilerplate is good, certainly.
i think greg got a lot of it recently though
a
if that's not your concern, then i don't fully understand why having more intrinsics is something we want to avoid at this stage
w
I don't think I said I was concerned about intrinsics?... I was concerned about syntax
a
interestingly, your message below is also concerned with my syntax and not my semantics. i am trying to find ways to implement interesting execution mechanisms. i would like to understand your concerns so that we can bridge them.
if i'm willing to write documentation and to own the code that implements the feature, would that address your concerns regarding a generic 
RetryableRequest
?
w
Doing some design would be the most helpful thing
a
i did. what format would you prefer?
w
Oh. Where?
One thing I just realized was that the only thing that is currently actually retriable is a process, because it's the only thing that isn't cached
So "generically retriable" would mean overhauling caching of nodes as well
a
i actually think the opposite might be true, in that process executions have their own special external cache, while "retrying" would only occur if there was a failure somewhere along the line of evaluating the real `Select`/`yield Get()` (and therefore it wouldn't be cached, unless you're saying we also cache nodes that raise an 
ExecutionError
?)
w
@aloof-angle-91616: failures are cached everywhere except with processes, currently
a
ok, so i'll figure out the changes needed to propagate that to other nodes and can report back
not like make a PR
just figuring it out
thanks!
w
if it were going to be "syntax", i think that coupling it to the kinds of changes suggested on that ticket would be helpful (ie, cacheability/side-effects).
but again, i don't think this is actually a blocker for a first version of test retry, and that's the thing we need most right now
a
that's correct, however this is the weekend
if it requires diving into cacheability in the engine already it would make sense to align it with the bullet points in that ticket for sure
w
yep! On that note, I'm going to disappear for a bit. Have a good one
a
thanks!!
Open in Slack
PreviousNext
Powered by