https://pantsbuild.org/ logo
Join Slack
Powered by
I wanted to get peoples' opinion about how to go a...
# development
h
I wanted to get peoples' opinion about how to go about implementing https://github.com/pantsbuild/pants/issues/7907
the most obvious way to introduce this kind of dynamic editing of workunit properties within the body of an 
@rule
seems to me to introduce a new, optional function parameter
of some special type 
WorkunitControl
đź‘Ť 1
this would have some methods on it that would allow the rule to affect the workunit (change the name, change the level, etc.)
to make this work, rule-parsing in python would have to recognize this special type, and treat it differently than other rule inputs. which I think is doable, but might be a bit messy in practice
and then the engine would have to get access to this value at workunit completion time so it could get access to whatever changes to the workunit were specified. again I think this is doable, but possibly messy
h
That makes sense to me, I think. It wouldn’t work to be a singleton like normal tho? You’d need to strip it from the rule signature?
h
@hundreds-father-404 yeah. my thinking is that the python rule parsing would strip that type, if it was present, from the rule signature before creating a Task
so 
def my_rule(a: Snapshot, b: WorkunitControl) -> Digest
would work exactly the same as 
def my_rule(a:Snapshot) -> Digest
w
@hundreds-breakfast-49010: i maybe shouldn’t have put both of those in the same ticket, but imo: the second part is more important.
and that part is clearer cut i think.
h
I think we need a construct like this to allow rules to change their own level. I don't see another way to do it offhand, anyway
other than modifying the return type
w
@hundreds-breakfast-49010: there is an idea in the second bullet.
A sketch of this might involve adding a Python interface that return types could implement that would allow them to affect their level. For example: 
LintResult
and 
TestResult
would implement the interface in order to render failing targets as 
error
.
h
ah, okay
yeah that's more what I wanted to talk about, what is the right construct to use to implement this feature (as well as other features that involve manipulation of the workunit)
I'm not sure offhand how to hook python-level manipulations of either a new optional input type, or the output type + some interface, to the engine. this might or might not be difficult I'm just not immeidately sure what code paths I'd need to modify
w
if we can avoid the APIs being imperative, that would be my preference. which is why i like the return types implementing an interface. it doesn’t address the first point, but i still think the first case can be done with templating.
h
that makes sense, yeah
w
@hundreds-breakfast-49010: it would be similar to the “is this a union” thing you did before, i think.
“does this return type implement ThisInterface”
h
I would like to do this via subclassing, but I don't know how easy it would be to make the engine pay attention to that
so the other alternative is looking for the presence of specific class or instance variables I suppose
w
those are similarly easy
because we have access to 
cpython
now
đź‘Ť 1
you could do either a check for presence of a field, or a cast to a particular type, and then a field lookup
i don’t even think we need to pass the type across the cffi boundary… could do a name lookup for the type
h
@witty-crayon-22786 it looks like the cpython API will let me check if a 
PyType
is a subclass of another `PyType`: https://docs.rs/cpython/0.5.0/cpython/struct.PyType.html#method.is_subtype_of
but that means I would need to get a reference to whichever special Python class is the one that allows return types to modify workunitproperties
the properties themselves can just be 
_
properties on the python object, I think. e.g. any python object that has 
_new_workunit_level
and is also a subclass of some python class 
ModifyWorkunit
we could also just check for specific named attributes like 
_new_workunit_level
, which has the (remote) risk of a name collision with a property a user cares about
w
you can also look up a type by name, i think.
imo, even if the rust side doesn’t actually check whether a type implements a particular interface, we should still have the interface
since it will be somewhat user visible.
class FmtResult(WorkunitResult)
(strawname)
the rust side could eagerly go looking for fields/methods without checking.
…oh. um, no. that’s not what i thought it was, heh.
Open in Slack
PreviousNext
Powered by