https://pantsbuild.org/ logo
Powered by
#development
Title
# development
h

hundreds-breakfast-49010

06/09/2020, 1:33 AM
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

hundreds-father-404

06/09/2020, 2:21 AM
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-breakfast-49010

06/09/2020, 2:26 AM
@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

witty-crayon-22786

06/09/2020, 2:27 AM
@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

hundreds-breakfast-49010

06/09/2020, 2:28 AM
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

witty-crayon-22786

06/09/2020, 2:28 AM
@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

hundreds-breakfast-49010

06/09/2020, 2:29 AM
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

witty-crayon-22786

06/09/2020, 2:31 AM
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

hundreds-breakfast-49010

06/09/2020, 2:31 AM
that makes sense, yeah
w

witty-crayon-22786

06/09/2020, 2:31 AM
@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

hundreds-breakfast-49010

06/09/2020, 2:37 AM
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

witty-crayon-22786

06/09/2020, 2:38 AM
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

hundreds-breakfast-49010

06/09/2020, 10:12 PM
@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

witty-crayon-22786

06/09/2020, 10:15 PM
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.
Powered by