Skip to content

Commit

Permalink
Clearer error for dependency conflicts (viamrobotics#4436)
Browse files Browse the repository at this point in the history
  • Loading branch information
cheukt authored Oct 10, 2024
1 parent fd3f342 commit c5d07b0
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 7 deletions.
10 changes: 5 additions & 5 deletions module/modmanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -834,11 +834,11 @@ func (mgr *Manager) newOnUnexpectedExitHandler(mod *module) func(exitCode int) b
if orphanedResourceNames := mgr.attemptRestart(mgr.restartCtx, mod); orphanedResourceNames != nil {
if mgr.removeOrphanedResources != nil {
mgr.removeOrphanedResources(mgr.restartCtx, orphanedResourceNames)
rNames := make([]string, 0, len(orphanedResourceNames))
for _, rName := range orphanedResourceNames {
rNames = append(rNames, rName.String())
}
mgr.logger.Debugw("Removed resources after failed module restart", "module", mod.cfg.Name, "resources", rNames)
mgr.logger.Debugw(
"Removed resources after failed module restart",
"module", mod.cfg.Name,
"resources", resource.NamesToStrings(orphanedResourceNames),
)
}
return false
}
Expand Down
9 changes: 9 additions & 0 deletions resource/name.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,12 @@ func (n Name) SDPTrackName() string {
func SDPTrackNameToShortName(name string) string {
return strings.ReplaceAll(name, "+", ":")
}

// NamesToStrings is a utility that takes a list of resource names and returns a list of fully qualified names.
func NamesToStrings(lst []Name) []string {
rNames := make([]string, 0, len(lst))
for _, rName := range lst {
rNames = append(rNames, rName.String())
}
return rNames
}
28 changes: 28 additions & 0 deletions resource/name_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,31 @@ func TestSDPTrackNameToShortName(t *testing.T) {
test.That(t, SDPTrackNameToShortName(tc.input), test.ShouldResemble, tc.output)
}
}

func TestNamesToStrings(t *testing.T) {
type testCase struct {
input []Name
output []string
}

camAPI := API{Type: APIType{Namespace: APINamespace("rdk"), Name: "component"}, SubtypeName: "camera"}
test.That(t, camAPI, test.ShouldResemble, APINamespaceRDK.WithComponentType("camera"))

tcs := []testCase{
{
input: []Name{},
output: []string{},
},
{
input: []Name{{API: camAPI, Remote: "", Name: "cam1"}},
output: []string{"rdk:component:camera/cam1"},
},
{
input: []Name{{API: camAPI, Remote: "", Name: "cam1"}, {API: camAPI, Remote: "abc", Name: "cam1"}},
output: []string{"rdk:component:camera/cam1", "rdk:component:camera/abc:cam1"},
},
}
for _, tc := range tcs {
test.That(t, NamesToStrings(tc.input), test.ShouldResemble, tc.output)
}
}
4 changes: 2 additions & 2 deletions resource/resource_graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,11 +595,11 @@ func (g *Graph) ResolveDependencies(logger logging.Logger) error {
default:
allErrs = multierr.Combine(
allErrs,
errors.Errorf("conflicting names for resource %q: %v", nodeName, nodeNames))
errors.Errorf("conflicting names for resource %q: %v", nodeName, NamesToStrings(nodeNames)))
logger.Errorw(
"cannot resolve dependency for resource due to multiple matching names",
"name", nodeName,
"conflicts", nodeNames,
"conflicts", NamesToStrings(nodeNames),
)
}
return Name{}, false
Expand Down

0 comments on commit c5d07b0

Please sign in to comment.