https://pantsbuild.org/ logo
Join Slack
Powered by
Do we actually still need the `@rule` decorator?
# development
a
Do we actually still need the 
@rule
decorator?
Thoughts: • All rules need to be registered using a function call - we don’t auto-discover them in code or similar • We no longer put type annotations on the decorator • Maybe it’s still useful documentation to show the boundaries of where `await Get`s are and aren’t allowed?
h
Counterpoint: we do put the 
name='run this rule'
though I think this is worth considering, although bias towards keeping it for documentation purposes.
Although maybe the name should come from the docstring? We do that in other places. Cc @hundreds-breakfast-49010. The downside would be that private rules could no longer have docstring or they’d be make public
a
Could use some tag in the docstring, like:
Copy code
"""
Uses Zinc to compile scala code to bytecode.

This rule values speed over correctness. Sometimes, things may go badly.

Display name: Compiling scala with zinc.
"""
Which is maybe less obvious or guided. But an option.
👍 1
h
Related to this rule name discussion is if there’s any mechanism for adding runtime information, like “running tests on foo”. I think there’s an open ticket but afk / going to doctor
h
what does getting rid of this decorator get us?
I don't want to overload arbitrary function docstrings with rule metadata
➕ 1
I'm actually okay with getting the display name or what have you from the docstring, but only if there's some kind of annotation like 
@rule
so that pants doesn't try to do this for every function
👍 1
a
Right now pants only considers things as rules if they’re in a call to 
rules()
in a 
register.py
So right now, to register a rule, you need to do two things: 1. Add a decorator 2. Reference the function name in a function call And those two things seem kind of redundant, when you could just do one or the other
a
also, the 
@rule
decorator means we do tons of stuff like 
ast
introspection and calling 
inspect.stack()
or whatever at import time
if we could do that later, we could reduce import time
not sure if that's a concern
the move to `async`/`await` means we don't need to check the rule for validity at all anymore (e.g. fixing broken 
return
or 
yield
calls)
👍 1
and i think this totally 100% jives with @hundreds-breakfast-49010's mention of keeping the 
@rule
decorator
and also means we can decouple the 
@rule
decorator (the process of attaching metadata to a rule) from the 
ast
manipulation process which is needed to use it in the engine
h
I would almost rather get rid of the necessity of explicitly registering rules in 
register.py
a
so the 
@rule
decorator can be easier to implement maybe
h
although that has its own can of worms I'm sure
a
i mean that's the thing
if the engine ensures exactly 1 path from subject to product, for all subjects and all products, and our error messages are good, that might be a good thing
i assume other dependency injection systems have already tried to solve this problem, with many different possible approaches
but we have a very powerful but also simple rule graph here which i think is a huge advantage
w
imo, 1) registration, and 2) marking something as a 
@rule
... are useful as seperate concepts
i want validation of my rules regardless of whether i use them in a particular context
👍 1
and i don't want them to be magically used in all contexts (especially if that means i need additional magic to prevent them from being registered... eg, in a unit test that is supposed to be isolated)
👍 1
h
I like the decorators for the proximity of knowing what is a normal function and what is a rule. For example, in the python test runner, we have a simple function to calculate the timeout and then lower below we have the rule. It’s nice to have the @rule to differentiate the two
a
I like the decorators for the proximity of knowing what is a normal function and what is a rule.
Why is this distinction important? imo it would be really nice to be able to transparently use any function, sync or async, as you naturally would in Python, and every place you actually care about whether it’s a rule or not is a usability bug… (ETA: Not sure how much I believe this vs devils advocating, I’ve felt both ways on this one in the past)
@witty-crayon-22786 Those messages explain why you like registration… Why do you like the decorator, separately?
h
For example, when debugging, reduce the surface area of where an engine failure could be coming from. Even though rules are modeled after normal pure functions, rules have extra magic. Dependency injection only works in rules, and I find the decorator helps me to understand “okay these parameters will be provided by the engine, not by a direct call site”
a
Those sound like Stockholm syndrome from the engine giving bad error messages? Imagine if the engine could give perfectly clear errors…
And imagine if dependency injection worked wherever you wanted it to… (ETA: Not sure how I actually feel about this one, but it’s an interesting thought experiment…)
w
I gave the reason for the decorator above: I'd like my code to be validated, even if it isn't registered anywhere
h
And imagine if dependency injection worked wherever you wanted it to…
I wouldn’t want this. It requires extra thinking for me to reason with dependency injection. I like the simplicity of normal pure python functions and how small the surface area is for understanding them. Dependency injection is neat, but extra magic, and we lose some ability to reason if now every single function might use that magic but might not
a
I gave the reason for the decorator above: I’d like my code to be validated, even if it isn’t registered anywhere
Aha! Somehow I negated something in there when I read it! What validation do we do on `@rule`s without knowing the full graph?
w
The validity of the parseability of your yield statements
Take a look at the implementation of the decorator
a
I wouldn’t want this. It requires extra thinking for me to reason with dependency injection. I like the simplicity of normal pure python functions and how small the surface area is for understanding them. Dependency injection is neat, but extra magic, and we lose some ability to reason if now every single function might use that magic but might not
Why is that different for rules? I understand that the transitive graph navigation thing is useful (“I have an Address, the rule for computing this needs a HydratedTarget, do the magic”), and I can see that dependency injection helps to make you not need to care about intermediate rules, but don’t you have the same complaint about extra arguments to rules you’re directly calling being magically populated by context? (Which is currently actually a source of real bugs…)
The validity of the parseability of your yield statements
Looking at the implementation, this feels like something that could maybe roughly be offloaded into the realms of MyPy? It’s effectively type-checking the arguments to 
Get
in a slightly meta-typey way?
h
@hundreds-father-404 agreed, I think we don't want dependency injection to just happen anywhere, that is hard to reason about
w
https://pantsbuild.slack.com/archives/C0D7TNJHL/p1578438299180400?thread_ts=1578406404.157000&cid=C0D7TNJHL yea, possibly. but 
@rule
as a marker on rules still seems useful, both as a marker, and as a place for additional arguments like 
name
to be specified with minimal magic.
👍 1
in code reviews, for example, i generally scroll till i see an 
@rule
, and then work from there into the rest (which are likely "private" functions, essentially... or at least implementation details of the rules)
👍 1
a
i don't think we need a decorator to do ast magic though?
if we want things to be private functions, we already have the trusty underscore
w
there is still "private" vs "public" vs "is a rule"
i didn't mean literally private... just that the 
@rule
and its exposed types are the actual API of an extension/plugin.
👍 1
a
yes
Open in Slack
PreviousNext
Powered by