Advisor: Replace Error naming with Failure (#100262)

This commit is contained in:
Andres Martinez Gotor
2025-02-07 15:48:18 +01:00
committed by GitHub
parent 0beb7e668b
commit f607bcd43f
12 changed files with 101 additions and 101 deletions
@@ -75,7 +75,7 @@ func (s *uidValidationStep) Description() string {
return "Check if the UID of each data source is valid."
}
func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) {
func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportFailure, error) {
ds, ok := i.(*datasources.DataSource)
if !ok {
return nil, fmt.Errorf("invalid item type %T", i)
@@ -83,8 +83,8 @@ func (s *uidValidationStep) Run(ctx context.Context, obj *advisor.CheckSpec, i a
// Data source UID validation
err := util.ValidateUID(ds.UID)
if err != nil {
return checks.NewCheckReportError(
advisor.CheckReportErrorSeverityLow,
return checks.NewCheckReportFailure(
advisor.CheckReportFailureSeverityLow,
fmt.Sprintf("Invalid UID '%s' for data source %s", ds.UID, ds.Name),
"Check the <a href='https://grafana.com/docs/grafana/latest/upgrade-guide/upgrade-v11.2/#grafana-data-source-uid-format-enforcement' target=_blank>documentation</a> for more information.",
s.ID(),
@@ -111,7 +111,7 @@ func (s *healthCheckStep) ID() string {
return "health-check"
}
func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) {
func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any) (*advisor.CheckReportFailure, error) {
ds, ok := i.(*datasources.DataSource)
if !ok {
return nil, fmt.Errorf("invalid item type %T", i)
@@ -132,8 +132,8 @@ func (s *healthCheckStep) Run(ctx context.Context, obj *advisor.CheckSpec, i any
}
resp, err := s.PluginClient.CheckHealth(ctx, req)
if err != nil || resp.Status != backend.HealthStatusOk {
return checks.NewCheckReportError(
advisor.CheckReportErrorSeverityHigh,
return checks.NewCheckReportFailure(
advisor.CheckReportFailureSeverityHigh,
fmt.Sprintf("Health check failed for %s", ds.Name),
fmt.Sprintf(
"Go to the <a href='/connections/datasources/edit/%s'>data source configuration</a>"+
@@ -14,7 +14,7 @@ import (
)
func TestCheck_Run(t *testing.T) {
t.Run("should return no errors when all datasources are healthy", func(t *testing.T) {
t.Run("should return no failures when all datasources are healthy", func(t *testing.T) {
datasources := []*datasources.DataSource{
{UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus"},
{UID: "valid-uid-2", Type: "mysql", Name: "MySQL"},
@@ -33,23 +33,23 @@ func TestCheck_Run(t *testing.T) {
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
items, err := check.Items(ctx)
assert.NoError(t, err)
errs := []advisor.CheckReportError{}
failures := []advisor.CheckReportFailure{}
for _, step := range check.Steps() {
for _, item := range items {
stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item)
stepFailures, err := step.Run(ctx, &advisor.CheckSpec{}, item)
assert.NoError(t, err)
if stepErr != nil {
errs = append(errs, *stepErr)
if stepFailures != nil {
failures = append(failures, *stepFailures)
}
}
}
assert.NoError(t, err)
assert.Equal(t, 2, len(items))
assert.Empty(t, errs)
assert.Empty(t, failures)
})
t.Run("should return errors when datasource UID is invalid", func(t *testing.T) {
t.Run("should return failures when datasource UID is invalid", func(t *testing.T) {
datasources := []*datasources.DataSource{
{UID: "invalid uid", Type: "prometheus", Name: "Prometheus"},
}
@@ -67,24 +67,24 @@ func TestCheck_Run(t *testing.T) {
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
items, err := check.Items(ctx)
assert.NoError(t, err)
errs := []advisor.CheckReportError{}
failures := []advisor.CheckReportFailure{}
for _, step := range check.Steps() {
for _, item := range items {
stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item)
stepFailures, err := step.Run(ctx, &advisor.CheckSpec{}, item)
assert.NoError(t, err)
if stepErr != nil {
errs = append(errs, *stepErr)
if stepFailures != nil {
failures = append(failures, *stepFailures)
}
}
}
assert.NoError(t, err)
assert.Equal(t, 1, len(items))
assert.Len(t, errs, 1)
assert.Equal(t, "Invalid UID 'invalid uid' for data source Prometheus", errs[0].Reason)
assert.Len(t, failures, 1)
assert.Equal(t, "Invalid UID 'invalid uid' for data source Prometheus", failures[0].Reason)
})
t.Run("should return errors when datasource health check fails", func(t *testing.T) {
t.Run("should return failures when datasource health check fails", func(t *testing.T) {
datasources := []*datasources.DataSource{
{UID: "valid-uid-1", Type: "prometheus", Name: "Prometheus"},
}
@@ -102,21 +102,21 @@ func TestCheck_Run(t *testing.T) {
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
items, err := check.Items(ctx)
assert.NoError(t, err)
errs := []advisor.CheckReportError{}
failures := []advisor.CheckReportFailure{}
for _, step := range check.Steps() {
for _, item := range items {
stepErr, err := step.Run(ctx, &advisor.CheckSpec{}, item)
stepFailures, err := step.Run(ctx, &advisor.CheckSpec{}, item)
assert.NoError(t, err)
if stepErr != nil {
errs = append(errs, *stepErr)
if stepFailures != nil {
failures = append(failures, *stepFailures)
}
}
}
assert.NoError(t, err)
assert.Equal(t, 1, len(items))
assert.Len(t, errs, 1)
assert.Equal(t, "Health check failed for Prometheus", errs[0].Reason)
assert.Len(t, failures, 1)
assert.Equal(t, "Health check failed for Prometheus", failures[0].Reason)
})
}
+1 -1
View File
@@ -25,5 +25,5 @@ type Step interface {
// Description returns the description of the step
Description() string
// Run executes the step for an item and returns a report
Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, item any) (*advisorv0alpha1.CheckReportError, error)
Run(ctx context.Context, obj *advisorv0alpha1.CheckSpec, item any) (*advisorv0alpha1.CheckReportFailure, error)
}
@@ -78,7 +78,7 @@ func (s *deprecationStep) ID() string {
return "deprecation"
}
func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, it any) (*advisor.CheckReportError, error) {
func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, it any) (*advisor.CheckReportFailure, error) {
p, ok := it.(pluginstore.Plugin)
if !ok {
return nil, fmt.Errorf("invalid item type %T", it)
@@ -96,8 +96,8 @@ func (s *deprecationStep) Run(ctx context.Context, _ *advisor.CheckSpec, it any)
return nil, nil
}
if i.Status == "deprecated" {
return checks.NewCheckReportError(
advisor.CheckReportErrorSeverityHigh,
return checks.NewCheckReportFailure(
advisor.CheckReportFailureSeverityHigh,
fmt.Sprintf("Plugin deprecated: %s", p.ID),
"Check the <a href='https://grafana.com/legal/plugin-deprecation/#a-plugin-i-use-is-deprecated-what-should-i-do' target=_blank>documentation</a> for recommended steps.",
s.ID(),
@@ -125,7 +125,7 @@ func (s *updateStep) ID() string {
return "update"
}
func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, i any) (*advisor.CheckReportError, error) {
func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, i any) (*advisor.CheckReportFailure, error) {
p, ok := i.(pluginstore.Plugin)
if !ok {
return nil, fmt.Errorf("invalid item type %T", i)
@@ -149,8 +149,8 @@ func (s *updateStep) Run(ctx context.Context, _ *advisor.CheckSpec, i any) (*adv
return nil, nil
}
if hasUpdate(p, info) {
return checks.NewCheckReportError(
advisor.CheckReportErrorSeverityLow,
return checks.NewCheckReportFailure(
advisor.CheckReportFailureSeverityLow,
fmt.Sprintf("New version available for %s", p.ID),
fmt.Sprintf(
"Go to the <a href='/plugins/%s?page=version-history'>plugin admin page</a>"+
@@ -21,12 +21,12 @@ func TestRun(t *testing.T) {
pluginArchives map[string]*repo.PluginArchiveInfo
pluginPreinstalled []string
pluginManaged []string
expectedErrors []advisor.CheckReportError
expectedFailures []advisor.CheckReportFailure
}{
{
name: "No plugins",
plugins: []pluginstore.Plugin{},
expectedErrors: []advisor.CheckReportError{},
name: "No plugins",
plugins: []pluginstore.Plugin{},
expectedFailures: []advisor.CheckReportFailure{},
},
{
name: "Deprecated plugin",
@@ -39,9 +39,9 @@ func TestRun(t *testing.T) {
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin1": {Version: "1.0.0"},
},
expectedErrors: []advisor.CheckReportError{
expectedFailures: []advisor.CheckReportFailure{
{
Severity: advisor.CheckReportErrorSeverityHigh,
Severity: advisor.CheckReportFailureSeverityHigh,
Reason: "Plugin deprecated: plugin1",
Action: "Check the <a href='https://grafana.com/legal/plugin-deprecation/#a-plugin-i-use-is-deprecated-what-should-i-do' target=_blank>documentation</a> for recommended steps.",
StepID: "deprecation",
@@ -60,9 +60,9 @@ func TestRun(t *testing.T) {
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin2": {Version: "1.1.0"},
},
expectedErrors: []advisor.CheckReportError{
expectedFailures: []advisor.CheckReportFailure{
{
Severity: advisor.CheckReportErrorSeverityLow,
Severity: advisor.CheckReportFailureSeverityLow,
Reason: "New version available for plugin2",
Action: "Go to the <a href='/plugins/plugin2?page=version-history'>plugin admin page</a> and upgrade to the latest version.",
StepID: "update",
@@ -81,9 +81,9 @@ func TestRun(t *testing.T) {
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin2": {Version: "beta"},
},
expectedErrors: []advisor.CheckReportError{
expectedFailures: []advisor.CheckReportFailure{
{
Severity: advisor.CheckReportErrorSeverityLow,
Severity: advisor.CheckReportFailureSeverityLow,
Reason: "New version available for plugin2",
Action: "Go to the <a href='/plugins/plugin2?page=version-history'>plugin admin page</a> and upgrade to the latest version.",
StepID: "update",
@@ -103,7 +103,7 @@ func TestRun(t *testing.T) {
"plugin3": {Version: "1.1.0"},
},
pluginPreinstalled: []string{"plugin3"},
expectedErrors: []advisor.CheckReportError{},
expectedFailures: []advisor.CheckReportFailure{},
},
{
name: "Managed plugin",
@@ -116,8 +116,8 @@ func TestRun(t *testing.T) {
pluginArchives: map[string]*repo.PluginArchiveInfo{
"plugin4": {Version: "1.1.0"},
},
pluginManaged: []string{"plugin4"},
expectedErrors: []advisor.CheckReportError{},
pluginManaged: []string{"plugin4"},
expectedFailures: []advisor.CheckReportFailure{},
},
}
@@ -134,19 +134,19 @@ func TestRun(t *testing.T) {
items, err := check.Items(context.Background())
assert.NoError(t, err)
errs := []advisor.CheckReportError{}
failures := []advisor.CheckReportFailure{}
for _, step := range check.Steps() {
for _, item := range items {
stepErr, err := step.Run(context.Background(), &advisor.CheckSpec{}, item)
stepFailures, err := step.Run(context.Background(), &advisor.CheckSpec{}, item)
assert.NoError(t, err)
if stepErr != nil {
errs = append(errs, *stepErr)
if stepFailures != nil {
failures = append(failures, *stepFailures)
}
}
}
assert.NoError(t, err)
assert.Equal(t, len(tt.plugins), len(items))
assert.Equal(t, tt.expectedErrors, errs)
assert.Equal(t, tt.expectedFailures, failures)
})
}
}
+4 -4
View File
@@ -4,14 +4,14 @@ import (
advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1"
)
func NewCheckReportError(
severity advisor.CheckReportErrorSeverity,
func NewCheckReportFailure(
severity advisor.CheckReportFailureSeverity,
reason string,
action string,
stepID string,
itemID string,
) *advisor.CheckReportError {
return &advisor.CheckReportError{
) *advisor.CheckReportFailure {
return &advisor.CheckReportFailure{
Severity: severity,
Reason: reason,
Action: action,