Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

Commit

Permalink
fix: Remove dead code (#419)
Browse files Browse the repository at this point in the history
* fix: Remove dead code (default error classifier)

The code inside the defaultErrorClassifier was never actually executed, because it was always passed a 'diag', but the first line in defaultErrorClassifier was "if err type is diag, return nil".

In any case, it's definitely better not to classify a single error twice. A more reasonable implementation might be to 'fall back to default classifier, if specific classifier returned nil', but for now, let's just remove it.

* test: Remove stale code
  • Loading branch information
shimonp21 authored Jul 13, 2022
1 parent 5c638bb commit 204eaf9
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 31 deletions.
14 changes: 0 additions & 14 deletions provider/execution/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,6 @@ const (

type ErrorClassifier func(meta schema.ClientMeta, resourceName string, err error) diag.Diagnostics

func defaultErrorClassifier(_ schema.ClientMeta, resourceName string, err error) diag.Diagnostics {
if _, ok := err.(diag.Diagnostic); ok {
return nil
}
if _, ok := err.(diag.Diagnostics); ok {
return nil
}
if strings.Contains(err.Error(), ": socket: too many open files") {
// Return a Diagnostic error so that it can be properly propagated back to the user via the CLI
return fromError(err, diag.WithResourceName(resourceName), diag.WithSummary(fdLimitMessage), diag.WithType(diag.THROTTLE), diag.WithSeverity(diag.WARNING))
}
return nil
}

func ClassifyError(err error, opts ...diag.BaseErrorOption) diag.Diagnostics {
if err != nil && strings.Contains(err.Error(), ": socket: too many open files") {
// Return a Diagnostic error so that it can be properly propagated back to the user via the CLI
Expand Down
28 changes: 14 additions & 14 deletions provider/execution/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ type TableExecutor struct {
Db Storage
// Logger associated with this execution
Logger hclog.Logger
// classifiers
classifiers []ErrorClassifier
// classifier
classifier ErrorClassifier
// metadata to be passed to each created resource in the execution, used by cq* resolvers.
metadata map[string]interface{}
// When the execution started
Expand All @@ -52,10 +52,6 @@ type TableExecutor struct {

// NewTableExecutor creates a new TableExecutor for given schema.Table
func NewTableExecutor(resourceName string, db Storage, logger hclog.Logger, table *schema.Table, metadata map[string]interface{}, classifier ErrorClassifier, goroutinesSem *semaphore.Weighted, timeout time.Duration) TableExecutor {
var classifiers = []ErrorClassifier{defaultErrorClassifier}
if classifier != nil {
classifiers = append([]ErrorClassifier{classifier}, classifiers...)
}
var c [2]schema.ColumnList
c[0], c[1] = db.Dialect().Columns(table).Sift()

Expand All @@ -65,7 +61,7 @@ func NewTableExecutor(resourceName string, db Storage, logger hclog.Logger, tabl
Db: db,
Logger: logger,
metadata: metadata,
classifiers: classifiers,
classifier: classifier,
executionStart: time.Now().Add(executionJitter),
columns: c,
goroutinesSem: goroutinesSem,
Expand Down Expand Up @@ -435,16 +431,20 @@ func (e TableExecutor) handleResolveError(meta schema.ClientMeta, r *schema.Reso
diag.WithSummary("failed to resolve table %q", e.Table.Name),
)...)

if e.classifier == nil {
return errAsDiags
}

classifiedDiags := make(diag.Diagnostics, 0, len(errAsDiags))
for _, c := range e.classifiers {
// fromError gives us diag.Diagnostics, but we need to make sure to pass one diag at a time to the classifiers and collect results,
// mostly because Unwrap()/errors.As() can't work on multiple diags
for _, d := range errAsDiags {
if diags := c(meta, e.ResourceName, d); diags != nil {
classifiedDiags = classifiedDiags.Add(diags)
}

// fromError gives us diag.Diagnostics, but we need to make sure to pass one diag at a time to the classifier and collect results,
// mostly because Unwrap()/errors.As() can't work on multiple diags
for _, d := range errAsDiags {
if diags := e.classifier(meta, e.ResourceName, d); diags != nil {
classifiedDiags = classifiedDiags.Add(diags)
}
}

if classifiedDiags.HasDiags() {
return classifiedDiags
}
Expand Down
3 changes: 0 additions & 3 deletions provider/execution/execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,9 +600,6 @@ func TestTableExecutor_Resolve(t *testing.T) {
if tc.SetupStorage != nil {
storage = tc.SetupStorage(t)
}
if tc.Name == "ignore_error_recursive" {
fmt.Println("debug")
}
limiter := semaphore.NewWeighted(int64(limit.GetMaxGoRoutines()))
exec := NewTableExecutor(tc.Name, storage, testlog.New(t), tc.Table, nil, nil, limiter, 10*time.Second)
count, diags := exec.Resolve(context.Background(), executionClient)
Expand Down

0 comments on commit 204eaf9

Please sign in to comment.