https://pantsbuild.org/ logo
Join Slack
Powered by
<@UB2J9BQA0> I updated <https://github.com/pantsbu...
# development
h
@hundreds-father-404 I updated https://github.com/pantsbuild/pants/pull/8922 in light of your recent changes to the name 
@goal_rule
@witty-crayon-22786
with respect to https://github.com/pantsbuild/pants/issues/6598, I think that we will want to have this PR merged regardless of what we ultimately do with rule caching - since no matter what the types that have this "uncacheable" annotation do work differently than other rule input types
👍 1
h
I agree. We’ve had it as a long-standing issue to not allow normal rules ot use 
Workspace
and 
Console
. This implements that
w
so, the semantics are: "if a type is marked uncacheable, it's only usable from `@goal_rule`" ?
how will this work if the implementation of #6598 results in a 
@rule(uncacheable=True)
flag, or allows uncacheable types to be used in rules other than 
@goal_rule
s?
... i have to be honest, i still think that it might be a better idea to finish #6598 before doing this.
because the semantics that are achieved by #6598 should probably drive the API that is exposed?
h
@witty-crayon-22786 if that's what the implementation of #6598 ended up doing, or if that sort of thing became possible after #6598, then we'd probably want to change the code in rules.py that implements the check
but we'd still want to have a variable called "uncacheable" that lives on certain types
so this commit is still useful in reifying the concept of an uncacheable type
w
but would you want to allow 
uncacheable
things deeper in the graph, but only allow 
sideeffecting
things in 
@goal_rule
s ...?
h
even if exactly how we handle uncacheable types is probably going to change later
right now I don't think we have a good notion of what things are 
sideeffecting
vs 
uncacheable
or even the need to create those two categories
w
hm. i thought that we had come to some conclusions on that yesterday
h
maybe; I'd have to dig back into that thread
what I want to do right now is mark 
Console
Workspace
InteractiveRunner
and (pretty soon) 
HttpRequester
with some class variable to mark them as special wrt cacheability
w
h
and what exactly we will do with that flag will probably change later as we implement #6598, but right now today the right behavior is to allow these types only as inputs to 
@goal_rule
w
basically: side-effecting things should only be in @goal_rules, afaict, but #6598 means that we'd allow uncachaeable things deeper in the graph
👍 1
h
okay, then maybe what we should do now is change the flag in this commit back to 
side_effecting
👍 1
and right now 
side_effecting
is the same thing as "uncacheable", but that may change soon
w
yea. i think so.
h
as we introduce a new notion of "uncahceable" things that are not side-effecting
ok I'll update the PR
w
thanks for the discussion.
h
h
thanks, you’ll want to update the PR title and description
👍 1
w
🚢 : but consider taking one last look at the actual API
h
any problem with an 
@side_effecting
annotation that just puts a 
_side_effecting
value on the class?
h
Yes, that’s exactly what I recommend 🙂 And fwit, I like that you use the underscore and that the field is private
👍 1
Open in Slack
PreviousNext
Powered by