Pants: @rule workunits should be able to include runtime information

[blorente]

In the context of the V2 UI, the only way we render Nodes is by having a static string representation of their arguments. We would like to redesign this, in order to allow:

• @rules (TaskNodes inside Rust) should maybe be able to define their own string representation. Ideally, they would like to be able to access their arguments to insert in their string representation (e.g. an @rule(CompiledScala, [ScalaTarget, Dependencies]) might want to be represented as "Compiling target <name of the target>"). This is a bit challenging because when the workunit for a @rule is created, we don't yet have its argument values... but it would still be possible to adjust the workunit name in the TaskNode code after the arguments are determined, here.
• Additionally, @rules with non-exception-but-still failure results would like to be able to change the level of their workunit to warning or error in order to affect rendering. 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.

Some related discussions around porting Workunits to v2: https://github.com/pantsbuild/pants/issues/7071

[stuhood]

Thanks, this looks like a good launching off point!

Ideally, they would like to be able to access their parameter to insert in their string representation (e.g. an @rule(CompiledScala, [ScalaTarget, Dependencies]) might want to be represented as "Compiling target ").

I like this. But a complicating factor is that a Node will not necessarily have the values of all of its arguments before it starts running: instead, the uniquely identifying Params for the Node are computed at rulegraph "compile time" (for example, the uniquely identifying Param in your example may just be an Address). See #7600.

This doesn't mean that we couldn't compose a static description string for the @rule with the __str__ of its Params though (or with some other well-known "display" implementation that we would look for on a type used as a Param).

[stuhood]

A large portion of this was tackled in #8592! The last portion appears to be the templating of arguments/params. cc @gshuflin

[Eric-Arellano]

Closing as mostly implemented by recent work on workunits like https://github.com/pantsbuild/pants/pull/8592.

[stuhood]

Reopening this, because it's not possible to template or otherwise affect rule names, and the more we use them, the more clear it is that they don't add much information without that.

[stuhood]

Updated the name and title to encapsulate two cases that had consensus in #9921.

cc @asherf

[stuhood]

@Eric-Arellano experienced a variant of this today: the Graph's cycle detection uses the Display representation of the nodes in the cycle, and the Params to the @rules that walk the dep graph got quite a bit larger recently (including the OptionsBootstrapper).

If we had something like the first item here, then those rules could have better string representations. Even a default string representation of rules that contained only their literal arguments would go a long way (because the arguments to a rule are likely smaller than the Params), even without users doing any customization.

[stuhood]

@gshuflin: Another thought here (related to my previous comment).

It's possible that rendering the Params of a Node _would_ actually make for a good default here if we apply a strategy similar to the EngineAware trait, that would opt params in and out of the description. Because rendering:

format!("@rule {}({})", task.task.display_info.name, task.params)

...is _almost_ a good representation. The issue is that it includes too much information. If we were to include/exclude types, and render a simplified version of the type, you might be able to get something like:

@rule coordinator_of_tests((OptionsBootstrapper(..), tests/python/pants_test/util:argutil))

[gshuflin]

@gshuflin: Another thought here (related to my previous comment).

It's possible that rendering the Params of a Node _would_ actually make for a good default here if we apply a strategy similar to the EngineAware trait, that would opt params in and out of the description. Because rendering:

format!("@rule {}({})", task.task.display_info.name, task.params)

...is _almost_ a good representation. The issue is that it includes too much information. If we were to include/exclude types, and render a simplified version of the type, you might be able to get something like:

@rule coordinator_of_tests((OptionsBootstrapper(..), tests/python/pants_test/util:argutil))

https://github.com/pantsbuild/pants/pull/10665 introduces a new debug_hint method for rule parameters that we now can leverage to build better parameter debug messages.

[stuhood]

Thanks @gshuflin!

IMO, the EngineAware types provide enough power to call this issue resolved. Great stuff!