https://pantsbuild.org/ logo
Join Slack
Powered by
does anyone know where the mapping of input types ...
# development
h
does anyone know where the mapping of input types to python arguments happens, when we invoke an 
@rule
?
a
<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
cool thanks
so, in that function, 
deps
is computed from the static argument list of the python in source code, right?
a
that’s correct
h
so there's no chacne that when we call it, we'll provide more/fewer arguments than the python function wanted?
a
“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
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
is there any reason that can’t be done by just having the rule request that type?
h
exactly, a rule would request something of type 
Workunit
just like any other type
a
and then perhaps check in the @rule decorator that only named rules can request that
h
but only if the rule is named
yeah
a
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
hm, interesting
a
i would need to read up on how we construct workunits currently to know if the await Get(...) thing is easy or not
h
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
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
right now we construct workunits in 
run
for the 
impl Node for NodeKey
block
a
ok
h
or, well, we construct them several places, for intrinsics, but that's the only place we construct them for `@rule`s
a
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
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
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
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
i'll check the issue
h
the parent_id stuff is making use of some task_local abstractions that I don't entirely understand
a
it also looks like they're all getting set just to have a single parent if i'm reading that issue correctly?
h
yeah
a
ok
h
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
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
looking at this now
Open in Slack
PreviousNext
Powered by