Skip to content

Commit

Permalink
Always use the workunit name in stacktraces (pantsbuild#10882)
Browse files Browse the repository at this point in the history
For stacktraces, it's not very helpful to use the `desc`. We instead want to always use the module name and rule name, which is more helpful for debugging.

This also changes the workunit name for `@goal_rule`s to default to their rule name, rather than the goal's name. This makes things more consistent with other rules. We still special case the `desc` code so that it renders that it's a goal.

Before:

```
Engine traceback:
  in select
  in `list` goal
  in pants.engine.internals.build_files.strip_address_origins
  in Find targets from input specs
  in pants.engine.internals.build_files.addresses_with_origins_from_address_specs
  in pants.engine.internals.build_files.resolve_address
```

After:

```
Engine traceback:
  in select
  in pants.backend.project_info.list_targets.list_targets
  in pants.engine.internals.build_files.strip_address_origins
  in pants.engine.internals.graph.resolve_addresses_with_origins
  in pants.engine.internals.build_files.addresses_with_origins_from_address_specs
  in pants.engine.internals.build_files.resolve_address
```

[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Sep 30, 2020
1 parent 4ffc6ae commit ec6492f
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 19 deletions.
6 changes: 3 additions & 3 deletions src/python/pants/engine/internals/scheduler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,17 +295,17 @@ def test_trace_includes_rule_exception_traceback(self):
assert_equal_with_printing(
self,
dedent(
"""\
f"""\
1 Exception encountered:
Engine traceback:
in select
in Nested raise
in {self.__module__}.{nested_raise.__name__}
Traceback (most recent call last):
File LOCATION-INFO, in nested_raise
fn_raises(b)
File LOCATION-INFO, in fn_raises
raise Exception(f"An exception for {type(x).__name__}")
raise Exception(f"An exception for {{type(x).__name__}}")
Exception: An exception for B
"""
),
Expand Down
12 changes: 5 additions & 7 deletions src/python/pants/engine/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,16 +273,14 @@ def rule_decorator(func, **kwargs) -> Callable:
is_goal_cls = issubclass(return_type, Goal)
validate_parameter_types(func_id, parameter_types, cacheable)

# Set a default canonical name if one is not explicitly provided. For Goal classes
# this is the name of the Goal; for other named ruled this is the module and name of the
# function that implements it.
effective_name = kwargs.get("canonical_name")
if effective_name is None:
effective_name = return_type.name if is_goal_cls else f"{func.__module__}.{func.__name__}"
# Set a default canonical name if one is not explicitly provided to the module and name of the
# function that implements it. This is used as the workunit name.
effective_name = kwargs.get("canonical_name", f"{func.__module__}.{func.__name__}")

# Set a default description, which is used in the dynamic UI and stacktraces.
effective_desc = kwargs.get("desc")
if effective_desc is None and is_goal_cls:
effective_desc = f"`{effective_name}` goal"
effective_desc = f"`{return_type.name}` goal"

effective_level = kwargs.get("level", LogLevel.TRACE)
if not isinstance(effective_level, LogLevel):
Expand Down
5 changes: 2 additions & 3 deletions src/python/pants/engine/rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,13 +456,12 @@ def test_bogus_rules(self):
def a_named_rule(a: int, b: str) -> bool:
return False

def test_goal_rule_automatically_gets_name_from_goal(self):
def test_goal_rule_automatically_gets_desc_from_goal(self):
@goal_rule
def some_goal_rule() -> Example:
return Example(exit_code=0)

name = some_goal_rule.rule.canonical_name
self.assertEqual(name, "example")
assert some_goal_rule.rule.desc == "`example` goal"

def test_can_override_goal_rule_name(self):
@goal_rule(canonical_name="some_other_name")
Expand Down
8 changes: 2 additions & 6 deletions src/rust/engine/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1174,9 +1174,7 @@ impl Node for NodeKey {
let workunit_name = self.workunit_name();
let failure_name = match &self {
NodeKey::Task(ref task) => {
let name = user_facing_name
.clone()
.unwrap_or_else(|| workunit_name.clone());
let name = workunit_name.clone();
let engine_aware_param_ty =
externs::type_for_type_id(context.core.types.engine_aware_parameter);
let displayable_param_names: Vec<_> = task
Expand Down Expand Up @@ -1211,9 +1209,7 @@ impl Node for NodeKey {
)
}
}
_ => user_facing_name
.clone()
.unwrap_or_else(|| workunit_name.clone()),
_ => workunit_name.clone(),
};

let metadata = WorkunitMetadata {
Expand Down

0 comments on commit ec6492f

Please sign in to comment.