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

hundreds-breakfast-49010

11/23/2019, 12:06 AM
does anyone know where the mapping of input types to python arguments happens, when we invoke an
@rule
?
a

aloof-angle-91616

11/23/2019, 12:07 AM
<http://nodes.rs|nodes.rs>
Copy code
impl WrappedNode for Task {
  type Item = Value;

  fn run(self, context: Context) -> NodeFuture<Value> {
    let params = self.params;
    let deps = {
      let edges = &context
        .core
        .rule_graph
        .edges_for_inner(&self.entry)
        .expect("edges for task exist.");
      future::join_all(
        self
          .task
          .clause
          .into_iter()
          .map(|s| Select::new_from_edges(params.clone(), s.product, edges).run(context.clone()))
          .collect::<Vec<_>>(),
      )
    };

    let func = self.task.func;
    let entry = self.entry;
    let product = self.product;
    deps
      .then(move |deps_result| match deps_result {
        Ok(deps) => externs::call(&externs::val_for(&func.0), &deps),
        Err(failure) => Err(failure),
      })
      .then(move |task_result| match task_result {
        Ok(val) => match externs::get_type_for(&val) {
          t if t == context.core.types.coroutine => Self::generate(context, params, entry, val),
          t if t == product => ok(val),
          _ => err(throw(&format!(
            "{:?} returned a result value that did not satisfy its constraints: {:?}",
            func, val
          ))),
        },
        Err(failure) => err(failure),
      })
      .to_boxed()
  }
}
externs::call(&externs::val_for(&func.0), &deps)
invokes the function
if it's an
async def
, that means it returns a
coroutine
so then we call
Self::generate()
and keep sending it back things until it completes
(i'm in my "remove all yield rules" branch, so there's no
context.core.types.generator
)
h

hundreds-breakfast-49010

11/23/2019, 12:11 AM
cool thanks
so, in that function,
deps
is computed from the static argument list of the python in source code, right?
a

aloof-angle-91616

11/23/2019, 12:16 AM
that’s correct
h

hundreds-breakfast-49010

11/23/2019, 12:16 AM
so there's no chacne that when we call it, we'll provide more/fewer arguments than the python function wanted?
a

aloof-angle-91616

11/23/2019, 12:18 AM
“no chance” is a strong word, not sure if we e.g. check for multiple args of the same type or what happens there
but basically, yes, since we take the type annotations from the method decl, we should be safe in assuming the number of args we have from the rule decl is correct
how to add multiple Params at once to an await Get(...) is left as an exercise for the reader
h

hundreds-breakfast-49010

11/23/2019, 12:19 AM
the context here is that we'd like to make it so that named rules can have an additional, optional, argument representing the workunit, so that python code can write metadata to it that shows up on the workunit
so I guess that would need to happen at rule static analysis time
a

aloof-angle-91616

11/23/2019, 12:20 AM
is there any reason that can’t be done by just having the rule request that type?
h

hundreds-breakfast-49010

11/23/2019, 12:20 AM
exactly, a rule would request something of type
Workunit
just like any other type
a

aloof-angle-91616

11/23/2019, 12:20 AM
and then perhaps check in the @rule decorator that only named rules can request that
h

hundreds-breakfast-49010

11/23/2019, 12:20 AM
but only if the rule is named
yeah
a

aloof-angle-91616

11/23/2019, 12:21 AM
i think that sounds like a good idea
it seems maybe a little magical, i might like to instead be able to explicitly request a workunit with a name in an
await Get(Workunit, WorkUnitRequest(name='idk'))
. i’m aware that’s not what we currently have, but it’s just an idea
i don’t have an issue with checking in the rule decorator
h

hundreds-breakfast-49010

11/23/2019, 12:24 AM
hm, interesting
a

aloof-angle-91616

11/23/2019, 12:24 AM
i would need to read up on how we construct workunits currently to know if the await Get(...) thing is easy or not
h

hundreds-breakfast-49010

11/23/2019, 12:24 AM
yeah I think this whole feature is a little magical, insofar as it'smeant to provide rules with a way to control engine internal stuff in pure python
a

aloof-angle-91616

11/23/2019, 12:25 AM
yes but i want that to happen more lol
and by magical i just meant the relationship between the name and the workunit
if the error message is good it should be fine
h

hundreds-breakfast-49010

11/23/2019, 12:25 AM
right now we construct workunits in
run
for the
impl Node for NodeKey
block
a

aloof-angle-91616

11/23/2019, 12:25 AM
ok
h

hundreds-breakfast-49010

11/23/2019, 12:25 AM
or, well, we construct them several places, for intrinsics, but that's the only place we construct them for `@rule`s
a

aloof-angle-91616

11/23/2019, 12:26 AM
then i wouldn’t recommend changing course for the await Get(...) thing right now
i’ll think about it and propose it later if it’s still interesting
thank you for telling me where to look!
h

hundreds-breakfast-49010

11/23/2019, 12:27 AM
what I was thinking is that, in that
run
impl, we'd need to have access to whatever metadata the rule-writer adds to this "magic" type, whether the rule gets it from a parameter from a
Get
a

aloof-angle-91616

11/23/2019, 12:29 AM
yes
hm
it doesn't seem incredibly difficult to add this as a
Get
if only because all we seem to need to do is to do this:
Copy code
let workunit = WorkUnit {
          name,
          time_span: TimeSpan::since(&start_time),
          span_id,
          // TODO: set parent_id with the proper value, issue #7969
          parent_id: None,
        };
        context2.session.workunit_store().add_workunit(workunit)
again, i would not recommend looking into this now, just thinking out loud
h

hundreds-breakfast-49010

11/23/2019, 12:51 AM
btw, do you have any thoughts on how to solve that TODO?
that's something that I think toolchain is going to have resolve as we make use of streaming workunits
a

aloof-angle-91616

11/23/2019, 12:52 AM
i'll check the issue
h

hundreds-breakfast-49010

11/23/2019, 12:52 AM
the parent_id stuff is making use of some task_local abstractions that I don't entirely understand
a

aloof-angle-91616

11/23/2019, 12:59 AM
it also looks like they're all getting set just to have a single parent if i'm reading that issue correctly?
h

hundreds-breakfast-49010

11/23/2019, 1:24 AM
yeah
a

aloof-angle-91616

11/23/2019, 1:24 AM
ok
h

hundreds-breakfast-49010

11/23/2019, 1:24 AM
it looks like there's some code in zipkin_reporter.py that is doing something to the parent_id
well after it exits the engine
a

aloof-angle-91616

11/25/2019, 2:38 AM
right
i can't tell if what it's doing right now is correct but hacky
or if it sets every parent id to just be the single one
i think the latter
./pants test tests/python/pants_test/engine:engine -- -vs -k test_async_reporting
continued to pass
h

hundreds-breakfast-49010

11/25/2019, 5:51 PM
looking at this now